From a0934f65d202e2410513f4c13007a65b1c91cd3d Mon Sep 17 00:00:00 2001 From: Pokorny Martin Date: Tue, 2 Jan 2024 11:51:16 +0100 Subject: [PATCH 1/7] NPE during the release of a lock --- .../actions/LockedResourcesBuildAction.java | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/jenkins/plugins/lockableresources/actions/LockedResourcesBuildAction.java b/src/main/java/org/jenkins/plugins/lockableresources/actions/LockedResourcesBuildAction.java index 477b7e8cc..7a5be5d3a 100644 --- a/src/main/java/org/jenkins/plugins/lockableresources/actions/LockedResourcesBuildAction.java +++ b/src/main/java/org/jenkins/plugins/lockableresources/actions/LockedResourcesBuildAction.java @@ -77,7 +77,12 @@ public static void updateAction(Run build, List resourceNames) { for (String name : resourceNames) { LockableResource r = LockableResourcesManager.get().fromName(name); - action.add(new ResourcePOJO(r)); + if (r != null) { + action.add(new ResourcePOJO(r)); + } else { + // probably a ephemeral resource has been deleted + action.add(new ResourcePOJO(name, "")); + } } } @@ -101,7 +106,11 @@ private void add(ResourcePOJO r) { @Restricted(NoExternalUse.class) public static LockedResourcesBuildAction fromResources(Collection resources) { List resPojos = new ArrayList<>(); - for (LockableResource r : resources) resPojos.add(new ResourcePOJO(r)); + for (LockableResource r : resources) { + if (r != null) { + resPojos.add(new ResourcePOJO(r)); + } + } return new LockedResourcesBuildAction(resPojos); } From 15b16e9c8b3ab73d09dea843580dbb61c1dc70a9 Mon Sep 17 00:00:00 2001 From: Pokorny Martin Date: Tue, 2 Jan 2024 14:49:09 +0100 Subject: [PATCH 2/7] API-properties --- .../jenkins/plugins/lockableresources/LockableResource.java | 3 ++- .../plugins/lockableresources/LockableResourceProperty.java | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java index 30a7e27d6..7aa9f1858 100644 --- a/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java +++ b/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java @@ -259,7 +259,7 @@ public List getLabelsAsList() { * @param labelToFind Label to find. * @return {@code true} if this resource contains the label. */ - @Exported + @Restricted(NoExternalUse.class) public boolean hasLabel(@CheckForNull String labelToFind) { return this.labelsContain(labelToFind); } @@ -446,6 +446,7 @@ public boolean isLocked() { * @return the lock cause or null if not locked */ @CheckForNull + @Exported public String getLockCause() { final DateFormat format = SimpleDateFormat.getDateTimeInstance(MEDIUM, SHORT); final String timestamp = (reservedTimestamp == null ? "" : format.format(reservedTimestamp)); diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java index 97b97791c..2647b623d 100644 --- a/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java +++ b/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java @@ -8,7 +8,9 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.export.Exported; +import org.kohsuke.stapler.export.ExportedBean; +@ExportedBean(defaultVisibility = 999) public class LockableResourceProperty extends AbstractDescribableImpl implements Serializable { From 8d51bd4a0ade4fa35455c2ba3a8729dacf0f1f8e Mon Sep 17 00:00:00 2001 From: Pokorny Martin Date: Fri, 5 Apr 2024 20:38:22 +0200 Subject: [PATCH 3/7] Adapt plugin for Data Table API 2.0.x --- .../tableLabels/table.jelly | 4 ++-- .../tableQueue/table.jelly | 4 ++-- .../tableResources/table.jelly | 22 +++---------------- 3 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableLabels/table.jelly b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableLabels/table.jelly index 63f44ff37..92107d2cd 100644 --- a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableLabels/table.jelly +++ b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableLabels/table.jelly @@ -32,7 +32,7 @@ THE SOFTWARE.
- + diff --git a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableQueue/table.jelly b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableQueue/table.jelly index 4f6231c00..468c79953 100644 --- a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableQueue/table.jelly +++ b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableQueue/table.jelly @@ -62,7 +62,7 @@ THE SOFTWARE.
${%labels.table.column.labels}${%labels.table.column.labels} ${%labels.table.column.assigned} ${%labels.table.column.free} ${%labels.table.column.percentage}
${%queue.table.column.action} - + diff --git a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableResources/table.jelly b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableResources/table.jelly index 3ae5ddfba..6b741f072 100644 --- a/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableResources/table.jelly +++ b/src/main/resources/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction/tableResources/table.jelly @@ -35,7 +35,7 @@ THE SOFTWARE.
${%queue.table.column.request.type} ${%queue.table.column.request.info}${%queue.table.column.reason}${%queue.table.column.reason} ${%queue.table.column.requested.by} ${%queue.table.column.requested.at} ${%queue.table.column.priority}
- - - - - - - - - - - - - - - - - + - + From eac5347d6d99bce57f1a183441d9baabedbf157d Mon Sep 17 00:00:00 2001 From: Martin Pokorny <89339813+mPokornyETM@users.noreply.github.com> Date: Mon, 22 Apr 2024 08:26:37 +0200 Subject: [PATCH 4/7] Update pom.xml --- pom.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pom.xml b/pom.xml index b9cd6a37b..1c136378e 100644 --- a/pom.xml +++ b/pom.xml @@ -86,6 +86,7 @@ io.jenkins.plugins data-tables-api + v2.0.3-1 org.jenkins-ci.plugins From 0ab384da1db09bf94ea8862067aa804abcda5839 Mon Sep 17 00:00:00 2001 From: Martin Pokorny <89339813+mPokornyETM@users.noreply.github.com> Date: Mon, 22 Apr 2024 08:37:50 +0200 Subject: [PATCH 5/7] Update pom.xml --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 1c136378e..f59aa5f16 100644 --- a/pom.xml +++ b/pom.xml @@ -86,7 +86,7 @@ io.jenkins.plugins data-tables-api - v2.0.3-1 + 2.0.3-1 org.jenkins-ci.plugins From 1056c0f89dd04383977d08924d734dc32b0a6708 Mon Sep 17 00:00:00 2001 From: Martin Pokorny <89339813+mPokornyETM@users.noreply.github.com> Date: Fri, 26 Apr 2024 19:37:56 +0200 Subject: [PATCH 6/7] Update pom.xml --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index f59aa5f16..164a70849 100644 --- a/pom.xml +++ b/pom.xml @@ -70,8 +70,8 @@ io.jenkins.tools.bom - bom-2.387.x - 2543.vfb_1a_5fb_9496d + bom-2.440.x + 2977.vdf61ecb_fb_e2d pom import From 89ac8999d9210bd8066661580e66a2b87d905929 Mon Sep 17 00:00:00 2001 From: Pokorny Martin Date: Fri, 26 Apr 2024 21:10:30 +0200 Subject: [PATCH 7/7] fix dependencies and tests --- pom.xml | 4 +- .../lockableresources/LockStepTest.java | 8 +-- ...ckStepTest_manualUnreserveUnblocksJob.java | 6 +- .../LockStepWithRestartTest.java | 4 +- .../LockableResourceApiTest.java | 5 +- .../lockableresources/TestHelpers.java | 57 +++++++++++++------ 6 files changed, 53 insertions(+), 31 deletions(-) diff --git a/pom.xml b/pom.xml index 164a70849..49e260462 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.jenkins-ci.plugins plugin - 4.80 + 4.81 @@ -86,7 +86,7 @@ io.jenkins.plugins data-tables-api - 2.0.3-1 + 2.0.5-1 org.jenkins-ci.plugins diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java index 4972e7d3c..0bd111b95 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest.java @@ -745,9 +745,8 @@ public void unlockButtonWithWaitingRuns() throws Exception { WorkflowJob p = j.jenkins.createProject(WorkflowJob.class, "p"); p.setDefinition(new CpsFlowDefinition("lock('resource1') { semaphore('wait-inside') }", true)); - JenkinsRule.WebClient wc = j.createWebClient(); - WorkflowRun prevBuild = null; + TestHelpers testHelpers = new TestHelpers(); for (int i = 0; i < 3; i++) { WorkflowRun rNext = p.scheduleBuild2(0).waitForStart(); LOGGER.info("start build " + rNext); @@ -756,11 +755,8 @@ public void unlockButtonWithWaitingRuns() throws Exception { j.waitForMessage("[resource1] is locked by build " + prevBuild.getFullDisplayName(), rNext); LOGGER.info("is paused " + rNext); isPaused(rNext, 1, 1); - // List resources = new ArrayList<>(); - // resources.add(LockableResourcesManager.get().fromName("resource1")); LOGGER.info("unlock resource1"); - // LockableResourcesManager.get().unlock(resources, null); - TestHelpers.clickButton(wc, "unlock"); + testHelpers.clickButton("unlock", "resource1"); } j.waitForMessage("Trying to acquire lock on [Resource: resource1]", rNext); diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest_manualUnreserveUnblocksJob.java b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest_manualUnreserveUnblocksJob.java index c44f06abb..d90e26e45 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest_manualUnreserveUnblocksJob.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockStepTest_manualUnreserveUnblocksJob.java @@ -25,9 +25,9 @@ public class LockStepTest_manualUnreserveUnblocksJob extends LockStepTestBase { @Test public void manualUnreserveUnblocksJob() throws Exception { LockableResourcesManager.get().createResource("resource1"); - JenkinsRule.WebClient wc = j.createWebClient(); - TestHelpers.clickButton(wc, "reserve"); + TestHelpers testHelpers = new TestHelpers(); + testHelpers.clickButton("reserve", "resource1"); LockableResource resource1 = LockableResourcesManager.get().fromName("resource1"); assertNotNull(resource1); resource1.setReservedBy("someone"); @@ -45,7 +45,7 @@ public void manualUnreserveUnblocksJob() throws Exception { WorkflowRun r = p.scheduleBuild2(0).waitForStart(); j.waitForMessage("[resource1] is not free, waiting for execution ...", r); j.assertLogNotContains("I am inside", r); - TestHelpers.clickButton(wc, "unreserve"); + testHelpers.clickButton("unreserve", "resource1"); j.waitForMessage("I am inside", r); j.assertLogContains("I am inside", r); j.assertBuildStatusSuccess(j.waitForCompletion(r)); diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockStepWithRestartTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockStepWithRestartTest.java index 5aad90593..338d79521 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockStepWithRestartTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockStepWithRestartTest.java @@ -180,7 +180,9 @@ public void checkQueueAfterRestart() throws Throwable { JenkinsRule.WebClient wc = j.createWebClient(); wc.login("user"); - TestHelpers.clickButton(wc, "unreserve"); + + TestHelpers testHelpers = new TestHelpers(); + testHelpers.clickButton("unreserve", "resource1"); lrm.unreserve(Collections.singletonList(lrm.fromName("resource1"))); diff --git a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java index 3a7a12666..bb547f8f0 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/LockableResourceApiTest.java @@ -37,9 +37,10 @@ public void reserveUnreserveApi() throws Exception { JenkinsRule.WebClient wc = j.createWebClient(); wc.login("user"); - TestHelpers.clickButton(wc, "reserve"); + TestHelpers testHelpers = new TestHelpers(); + testHelpers.clickButton("reserve", "a1"); assertThat(LockableResourcesManager.get().fromName("a1").isReserved(), is(true)); - TestHelpers.clickButton(wc, "unreserve"); + testHelpers.clickButton("unreserve", "a1"); assertThat(LockableResourcesManager.get().fromName("a1").isReserved(), is(false)); } diff --git a/src/test/java/org/jenkins/plugins/lockableresources/TestHelpers.java b/src/test/java/org/jenkins/plugins/lockableresources/TestHelpers.java index e14cb8fd5..4df135a10 100644 --- a/src/test/java/org/jenkins/plugins/lockableresources/TestHelpers.java +++ b/src/test/java/org/jenkins/plugins/lockableresources/TestHelpers.java @@ -8,19 +8,21 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; +import static org.mockito.Mockito.when; import hudson.model.FreeStyleProject; import hudson.model.Queue; import java.io.IOException; -import java.util.List; import java.util.logging.Logger; import jenkins.model.Jenkins; import net.sf.json.JSONArray; import net.sf.json.JSONObject; -import org.htmlunit.html.HtmlElement; -import org.htmlunit.html.HtmlElementUtil; -import org.htmlunit.html.HtmlPage; +import org.jenkins.plugins.lockableresources.actions.LockableResourcesRootAction; import org.jvnet.hudson.test.JenkinsRule; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; public final class TestHelpers { @@ -29,8 +31,18 @@ public final class TestHelpers { private static final int SLEEP_TIME = 100; private static final int MAX_WAIT = 5000; + @Mock + private StaplerRequest req; + + @Mock + private StaplerResponse rsp; + + private AutoCloseable mocks; + // Utility class - private TestHelpers() {} + public TestHelpers() { + this.mocks = MockitoAnnotations.openMocks(this); + } public static void waitForQueue(Jenkins jenkins, FreeStyleProject job) throws InterruptedException { waitForQueue(jenkins, job, Queue.Item.class); @@ -71,20 +83,31 @@ public static JSONObject getApiData(JenkinsRule rule) throws IOException { return rule.getJSON("plugin/lockable-resources/api/json").getJSONObject(); } - // Currently assumes one resource or only clicks the button for the first resource - public static void clickButton(JenkinsRule.WebClient wc, String action) throws Exception { - // disable exceptions, otherwise it will not parse jQuery scripts (used ba DataTable plugin) - wc.getOptions().setThrowExceptionOnScriptError(false); - HtmlPage htmlPage = wc.goTo("lockable-resources"); - List allButtons = htmlPage.getDocumentElement().getElementsByTagName("button"); + /** SImulate the click on the button in the LRM page + * note: Currently does not click on the button. Just simulate the doAction (stapler request) + * on the given resource. + * We shall provide some better solution like selenium tests. But for now it is fine. + */ + public void clickButton(String action, String resourceName) throws Exception { + LOGGER.info(action + " on " + resourceName); + LockableResourcesRootAction doAction = new LockableResourcesRootAction(); + when(req.getMethod()).thenReturn("POST"); + when(req.getParameter("resource")).thenReturn(resourceName); - HtmlElement reserveButton = null; - for (HtmlElement b : allButtons) { - String onClick = b.getAttribute("onClick"); - if (onClick != null && onClick.contains(action)) { - reserveButton = b; + switch (action) { + case "reserve": { + doAction.doReserve(req, rsp); + break; + } + case "unreserve": { + doAction.doUnreserve(req, rsp); + break; + } + case "unlock": { + LOGGER.info("doUnlock"); + doAction.doUnlock(req, rsp); + break; } } - HtmlElementUtil.click(reserveButton); } }
${%resources.table.column.index}${%resources.table.column.resource}${%resources.table.column.resource} ${%resources.table.column.status} ${%resources.table.column.timestamp}${%resources.table.column.labels}${%resources.table.column.labels} ${%resources.table.column.properties} ${%resources.table.column.action}