Skip to content

Commit

Permalink
Eliminate multiplicate code in LockableResourcesRootAction (#451)
Browse files Browse the repository at this point in the history
  • Loading branch information
@mPokornyETM
mPokornyETM authored Jan 31, 2023
1 parent c42f0df commit 6b7b959
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 47 deletions.
15 changes: 15 additions & 0 deletions src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import org.jenkins.plugins.lockableresources.queue.QueuedContextStruct;
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript;
import org.jenkinsci.plugins.workflow.steps.StepContext;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.StaplerRequest;

Expand Down Expand Up @@ -610,6 +612,19 @@ public synchronized void unlockNames(
save();
}

/** Returns names (IDs) of given *resources*.
*/
@Restricted(NoExternalUse.class)
public static List<String> getResourcesNames(List<LockableResource> resources) {
List<String> resourceNames = new ArrayList<>();
if (resources != null) {
for (LockableResource resource : resources) {
resourceNames.add(resource.getName());
}
}
return resourceNames;
}

/** @see #getNextQueuedContext(List, List, boolean, QueuedContextStruct) */
@CheckForNull
private QueuedContextStruct getNextQueuedContext(
Expand Down
90 changes: 43 additions & 47 deletions src/main/java/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,11 @@ public void doUnlock(StaplerRequest req, StaplerResponse rsp)
{
Jenkins.get().checkPermission(UNLOCK);

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
LockableResourcesManager.get().unlock(resources, null);

rsp.forwardToPreviousPage(req);
Expand All @@ -184,19 +180,15 @@ public void doReserve(StaplerRequest req, StaplerResponse rsp)
{
Jenkins.get().checkPermission(RESERVE);

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
String userName = getUserName();
if (userName != null) {
if (!LockableResourcesManager.get().reserve(resources, userName)) {
rsp.sendError(423, Messages.error_resourceAlreadyLocked(name));
rsp.sendError(423, Messages.error_resourceAlreadyLocked(LockableResourcesManager.getResourcesNames(resources)));
return;
}
}
Expand All @@ -209,15 +201,11 @@ public void doSteal(StaplerRequest req, StaplerResponse rsp)
{
Jenkins.get().checkPermission(STEAL);

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
String userName = getUserName();
if (userName != null) {
LockableResourcesManager.get().steal(resources, userName);
Expand All @@ -239,23 +227,21 @@ public void doReassign(StaplerRequest req, StaplerResponse rsp)
throw new AccessDeniedException3(Jenkins.getAuthentication2(), STEAL);
}

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

if (userName.equals(r.getReservedBy())) {
// Can not achieve much by re-assigning the
// resource I already hold to myself again,
// that would just burn the compute resources.
// ...unless something catches the event? (TODO?)
return;
for (LockableResource resource : resources) {
if (userName.equals(resource.getReservedBy())) {
// Can not achieve much by re-assigning the
// resource I already hold to myself again,
// that would just burn the compute resources.
// ...unless something catches the event? (TODO?)
return;
}
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
LockableResourcesManager.get().reassign(resources, userName);

rsp.forwardToPreviousPage(req);
Expand All @@ -267,22 +253,20 @@ public void doUnreserve(StaplerRequest req, StaplerResponse rsp)
{
Jenkins.get().checkPermission(RESERVE);

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

String userName = getUserName();
if ((userName == null || !userName.equals(r.getReservedBy()))
&& !Jenkins.get().hasPermission(Jenkins.ADMINISTER)
) {
throw new AccessDeniedException3(Jenkins.getAuthentication2(), RESERVE);
for (LockableResource resource : resources) {
if ((userName == null || !userName.equals(resource.getReservedBy()))
&& !Jenkins.get().hasPermission(Jenkins.ADMINISTER)
) {
throw new AccessDeniedException3(Jenkins.getAuthentication2(), RESERVE);
}
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
LockableResourcesManager.get().unreserve(resources);

rsp.forwardToPreviousPage(req);
Expand All @@ -295,15 +279,11 @@ public void doReset(StaplerRequest req, StaplerResponse rsp)
Jenkins.get().checkPermission(UNLOCK);
// Should this also be permitted by "STEAL"?..

String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
List<LockableResource> resources = this.getResourcesFromRequest(req, rsp);
if (resources == null) {
return;
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
LockableResourcesManager.get().reset(resources);

rsp.forwardToPreviousPage(req);
Expand Down Expand Up @@ -333,4 +313,20 @@ public void doSaveNote(final StaplerRequest req, final StaplerResponse rsp)
rsp.forwardToPreviousPage(req);
}
}

private List<LockableResource> getResourcesFromRequest(final StaplerRequest req, final StaplerResponse rsp) throws IOException, ServletException {
// todo, when you try to improve the API to use multiple resources (a list instead of single one)
// this will be the best place to change it. Probably it will be enough to add a code piece here
// like req.getParameter("resources"); And split the content by some delimiter like ' ' (space)
String name = req.getParameter("resource");
LockableResource r = LockableResourcesManager.get().fromName(name);
if (r == null) {
rsp.sendError(404, Messages.error_resourceDoesNotExist(name));
return null;
}

List<LockableResource> resources = new ArrayList<>();
resources.add(r);
return resources;
}
}

0 comments on commit 6b7b959

Please sign in to comment.