Improve lockable-resources build (job) page

src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java

@@ -41,25 +41,28 @@ public LockStepExecution(LockStep step, StepContext context) {
 
     @Override
     public boolean start() throws Exception {
-        step.validate();
-
         // normally it might raise a exception, but we check it in the function .validate()
         // therefore we can skip the try-catch here.
         ResourceSelectStrategy resourceSelectStrategy =
                 ResourceSelectStrategy.valueOf(step.resourceSelectStrategy.toUpperCase(Locale.ENGLISH));
 
-        getContext().get(FlowNode.class).addAction(new PauseAction("Lock"));
         PrintStream logger = getContext().get(TaskListener.class).getLogger();
 
         Run<?, ?> run = getContext().get(Run.class);
-        LockableResourcesManager.printLogs("Trying to acquire lock on [" + step + "]", Level.FINE, LOGGER, logger);
 
         List<LockableResourcesStruct> resourceHolderList = new ArrayList<>();
 
-        LockableResourcesManager lrm = LockableResourcesManager.get();
         List<LockableResource> available = null;
-        LinkedHashMap<String, List<LockableResourceProperty>> resourceNames = new LinkedHashMap<>();
+        LinkedHashMap<String, List<LockableResourceProperty>> lockedResources = new LinkedHashMap<>();
+        LockableResourcesManager lrm = LockableResourcesManager.get();
         synchronized (lrm.syncResources) {
+            step.validate();
+
+            LockableResourcesManager.printLogs("Trying to acquire lock on [" + step + "]", Level.FINE, LOGGER, logger);
+
+            getContext().get(FlowNode.class).addAction(new PauseAction("Lock"));
+
+            List<String> resourceNames = new ArrayList<>();
             for (LockStepResource resource : step.getResources()) {
                 List<String> resources = new ArrayList<>();
                 if (resource.resource != null) {
@@ -71,10 +74,15 @@ public boolean start() throws Exception {
                                 logger);
                     }
                     resources.add(resource.resource);
+                    resourceNames.addAll(resources);
+                } else {
+                    resourceNames.add("N/A");
                 }
                 resourceHolderList.add(new LockableResourcesStruct(resources, resource.label, resource.quantity));
             }
 
+            LockedResourcesBuildAction.addLog(run, resourceNames, "try", step.toString());
+
             // determine if there are enough resources available to proceed
             available = lrm.getAvailableResources(resourceHolderList, logger, resourceSelectStrategy);
             if (available == null || available.isEmpty()) {
@@ -95,10 +103,10 @@ public boolean start() throws Exception {
             // since LockableResource contains transient variables, they cannot be correctly serialized
             // hence we use their unique resource names and properties
             for (LockableResource resource : available) {
-                resourceNames.put(resource.getName(), resource.getProperties());
+                lockedResources.put(resource.getName(), resource.getProperties());
             }
+            LockStepExecution.proceed(lockedResources, getContext(), step.toString(), step.variable);
         }
-        LockStepExecution.proceed(resourceNames, getContext(), step.toString(), step.variable);
 
         return false;
     }
@@ -166,11 +174,12 @@ public static void proceed(
         }
 
         try {
-
-            LockedResourcesBuildAction.updateAction(build, new ArrayList<>(lockedResources.keySet()));
+            List<String> resourceNames = new ArrayList<>(lockedResources.keySet());
+            final String resourceNamesAsString = String.join(",", lockedResources.keySet());
+            LockedResourcesBuildAction.addLog(build, resourceNames, "acquired", resourceDescription);
             PauseAction.endCurrentPause(node);
-            BodyInvoker bodyInvoker = context.newBodyInvoker()
-                    .withCallback(new Callback(new ArrayList<>(lockedResources.keySet()), resourceDescription));
+            BodyInvoker bodyInvoker =
+                    context.newBodyInvoker().withCallback(new Callback(resourceNames, resourceDescription));
             if (variable != null && !variable.isEmpty()) {
                 // set the variable for the duration of the block
                 bodyInvoker.withContext(
@@ -180,8 +189,7 @@ public static void proceed(
                             @Override
                             public void expand(@NonNull EnvVars env) {
                                 final LinkedHashMap<String, String> variables = new LinkedHashMap<>();
-                                final String resourceNames = String.join(",", lockedResources.keySet());
-                                variables.put(variable, resourceNames);
+                                variables.put(variable, resourceNamesAsString);
                                 int index = 0;
                                 for (Entry<String, List<LockableResourceProperty>> lockResourceEntry :
                                         lockedResources.entrySet()) {
@@ -222,9 +230,11 @@ private static final class Callback extends BodyExecutionCallback.TailCall {
 
         @Override
         protected void finished(StepContext context) throws Exception {
-            LockableResourcesManager.get().unlockNames(this.resourceNames, context.get(Run.class));
+            Run<?, ?> build = context.get(Run.class);
+            LockedResourcesBuildAction.addLog(build, this.resourceNames, "released", this.resourceDescription);
+            LockableResourcesManager.get().unlockNames(this.resourceNames, build);
             LockableResourcesManager.printLogs(
-                    "Lock released on resource [" + resourceDescription + "]",
+                    "Lock released on resource [" + this.resourceDescription + "]",
                     Level.FINE,
                     LOGGER,
                     context.get(TaskListener.class).getLogger());

src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java

@@ -494,14 +494,18 @@ public String getLockCauseDetail() {
         return build;
     }
 
+    // ---------------------------------------------------------------------------
     @Exported
     public String getBuildName() {
         if (getBuild() != null) return getBuild().getFullDisplayName();
         else return null;
     }
 
+    // ---------------------------------------------------------------------------
     public void setBuild(@Nullable Run<?, ?> lockedBy) {
+
         this.build = lockedBy;
+
         if (lockedBy != null) {
             this.buildExternalizableId = lockedBy.getExternalizableId();
             setReservedTimestamp(new Date());

src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java

@@ -41,6 +41,7 @@
 import jenkins.util.SystemProperties;
 import net.sf.json.JSONObject;
 import org.apache.commons.lang.StringUtils;
+import org.jenkins.plugins.lockableresources.actions.LockedResourcesBuildAction;
 import org.jenkins.plugins.lockableresources.queue.LockableResourcesStruct;
 import org.jenkins.plugins.lockableresources.queue.QueuedContextStruct;
 import org.jenkins.plugins.lockableresources.util.Constants;
@@ -106,9 +107,10 @@
 
     // ---------------------------------------------------------------------------
     /** Get declared resources, means only defined in config file (xml or JCaC yaml). */
+    @Restricted(NoExternalUse.class)
     public List<LockableResource> getDeclaredResources() {
         ArrayList<LockableResource> declaredResources = new ArrayList<>();
-        for (LockableResource r : this.getReadOnlyResources()) {
+        for (LockableResource r : this.getResources()) {
             if (!r.isEphemeral() && !r.isNodeResource()) {
                 declaredResources.add(r);
             }
@@ -165,7 +167,7 @@
     @Restricted(NoExternalUse.class)
     public List<LockableResource> getResourcesFromProject(String fullName) {
         List<LockableResource> matching = new ArrayList<>();
-        for (LockableResource r : this.getReadOnlyResources()) {
+        for (LockableResource r : this.getResources()) {
             String rName = r.getQueueItemProject();
             if (rName != null && rName.equals(fullName)) {
                 matching.add(r);
@@ -174,20 +176,6 @@
         return matching;
     }
 
-    // ---------------------------------------------------------------------------
-    /** Get all resources used by build. */
-    @Restricted(NoExternalUse.class)
-    public List<LockableResource> getResourcesFromBuild(Run<?, ?> build) {
-        List<LockableResource> matching = new ArrayList<>();
-        for (LockableResource r : this.getReadOnlyResources()) {
-            Run<?, ?> rBuild = r.getBuild();
-            if (rBuild != null && rBuild == build) {
-                matching.add(r);
-            }
-        }
-        return matching;
-    }
-
     // ---------------------------------------------------------------------------
     /**
      * Check if the label is valid. Valid in this context means, if is configured on someone resource.
@@ -198,9 +186,11 @@
             return false;
         }
 
-        for (LockableResource r : this.getReadOnlyResources()) {
-            if (r != null && r.isValidLabel(label)) {
-                return true;
+        synchronized (this.syncResources) {
+            for (LockableResource r : this.getResources()) {
+                if (r != null && r.isValidLabel(label)) {
+                    return true;
+                }
             }
         }
 
@@ -326,8 +316,10 @@
 
         if (resourceName != null) {
 
-            for (LockableResource r : this.getReadOnlyResources()) {
-                if (resourceName.equals(r.getName())) return r;
+            synchronized (this.syncResources) {
+                for (LockableResource r : this.getResources()) {
+                    if (resourceName.equals(r.getName())) return r;
+                }
             }
         } else {
             LOGGER.warning("Internal failure, fromName is empty or null:" + getStack());
@@ -590,47 +582,51 @@
 
     // ---------------------------------------------------------------------------
     /** Try to lock the resource and return true if locked. */
-    public boolean lock(List<LockableResource> resources, Run<?, ?> build) {
+    public boolean lock(List<LockableResource> resourcesToLock, Run<?, ?> build) {
 
-        LOGGER.fine("lock it: " + resources + " for build " + build);
+        LOGGER.fine("lock it: " + resourcesToLock + " for build " + build);
 
         if (build == null) {
-            LOGGER.warning("lock() will fails, because the build does not exits. " + resources);
+            LOGGER.warning("lock() will fails, because the build does not exits. " + resourcesToLock);
             return false; // not locked
         }
 
-        String cause = getCauses(resources);
+        String cause = getCauses(resourcesToLock);
         if (!cause.isEmpty()) {
             LOGGER.warning("lock() for build " + build + " will fails, because " + cause);
             return false; // not locked
         }
 
-        for (LockableResource r : resources) {
+        for (LockableResource r : resourcesToLock) {
             r.unqueue();
             r.setBuild(build);
         }
 
+        LockedResourcesBuildAction.findAndInitAction(build).addUsedResources(getResourcesNames(resourcesToLock));
+
         save();
 
         return true;
     }
 
     // ---------------------------------------------------------------------------
-    private void freeResources(List<LockableResource> unlockResources, @Nullable Run<?, ?> build) {
+    private void freeResources(List<LockableResource> unlockResources, Run<?, ?> build) {
 
         LOGGER.fine("free it: " + unlockResources);
 
         // make sure there is a list of resource names to unlock
-        if (unlockResources == null || unlockResources.isEmpty()) {
+        if (unlockResources == null || unlockResources.isEmpty() || build == null) {
             return;
         }
 
         List<LockableResource> toBeRemoved = new ArrayList<>();
+
         for (LockableResource resource : unlockResources) {
             // No more contexts, unlock resource
-            if (build != null && build != resource.getBuild()) {
-                continue; // this happens, when you push the unlock button in LRM page
-            }
+
+            // the resource has been currently unlocked (like by LRM page - button unlock, or by API)
+            if (!build.equals(resource.getBuild())) continue;
+
             resource.unqueue();
             resource.setBuild(null);
             uncacheIfFreeing(resource, true, false);
@@ -640,43 +636,53 @@
                 toBeRemoved.add(resource);
             }
         }
+
+        LockedResourcesBuildAction.findAndInitAction(build).removeUsedResources(getResourcesNames(unlockResources));
+
         // remove all ephemeral resources
         removeResources(toBeRemoved);
     }
 
-    // ---------------------------------------------------------------------------
-    @Deprecated
-    @ExcludeFromJacocoGeneratedReport
-    public void unlock(List<LockableResource> resourcesToUnLock, @Nullable Run<?, ?> build) {
-        List<String> resourceNamesToUnLock = LockableResourcesManager.getResourcesNames(resourcesToUnLock);
-        this.unlockNames(resourceNamesToUnLock, build);
-    }
+    public void unlockBuild(@Nullable Run<?, ?> build) {
 
-    // ---------------------------------------------------------------------------
-    @Deprecated
-    @ExcludeFromJacocoGeneratedReport
-    public void unlock(
-            @Nullable List<LockableResource> resourcesToUnLock, @Nullable Run<?, ?> build, boolean inversePrecedence) {
-        unlock(resourcesToUnLock, build);
-    }
+        if (build == null) {
+            return;
+        }
 
-    @Deprecated
-    @ExcludeFromJacocoGeneratedReport
-    public void unlockNames(
-            @Nullable List<String> resourceNamesToUnLock, @Nullable Run<?, ?> build, boolean inversePrecedence) {
-        this.unlockNames(resourceNamesToUnLock, build);
+        List<String> resourcesInUse =
+                LockedResourcesBuildAction.findAndInitAction(build).getCurrentUsedResourceNames();
+
+        if (resourcesInUse.size() == 0) {
+            return;
+        }
+        unlockNames(resourcesInUse, build);
     }
+
     // ---------------------------------------------------------------------------
     @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "not sure which exceptions might be catch.")
-    public void unlockNames(@Nullable List<String> resourceNamesToUnLock, @Nullable Run<?, ?> build) {
+    public void unlockNames(@Nullable List<String> resourceNamesToUnLock, Run<?, ?> build) {
 
         // make sure there is a list of resource names to unlock
         if (resourceNamesToUnLock == null || resourceNamesToUnLock.isEmpty()) {
             return;
         }
+        synchronized (this.syncResources) {
+            unlockResources(this.fromNames(resourceNamesToUnLock), build);
+        }
+    }
 
+    // ---------------------------------------------------------------------------
+    public void unlockResources(List<LockableResource> resourcesToUnLock) {
+        unlockResources(resourcesToUnLock, resourcesToUnLock.get(0).getBuild());
+    }
+
+    // ---------------------------------------------------------------------------
+    public void unlockResources(List<LockableResource> resourcesToUnLock, Run<?, ?> build) {
+        if (resourcesToUnLock == null || resourcesToUnLock.isEmpty()) {
+            return;
+        }
         synchronized (this.syncResources) {
-            this.freeResources(this.fromNames(resourceNamesToUnLock), build);
+            this.freeResources(resourcesToUnLock, build);
 
             while (proceedNextContext()) {
                 // process as many contexts as possible
@@ -913,8 +919,7 @@
                 r.setReservedBy(userName);
                 r.setStolen();
             }
-            unlock(resources, null, false);
-            // unlock() nulls resource.reservedTimestamp via resource.setBuild(null), so set it afterwards
+            unlockResources(resources);
             Date date = new Date();
             for (LockableResource r : resources) {
                 r.setReservedTimestamp(date);
@@ -999,7 +1004,7 @@
         synchronized (this.syncResources) {
             // Not calling reset() because that also un-queues the resource
             // and we want to proclaim it is usable (if anyone is waiting)
-            this.unlock(resources, null);
+            this.unlockResources(resources);
             this.unreserve(resources);
         }
     }

src/main/java/org/jenkins/plugins/lockableresources/actions/LockableResourcesRootAction.java

@@ -521,7 +521,7 @@ public void doUnlock(StaplerRequest req, StaplerResponse rsp) throws IOException
             return;
         }
 
-        LockableResourcesManager.get().unlock(resources, null);
+        LockableResourcesManager.get().unlockResources(resources);
 
         rsp.forwardToPreviousPage(req);
     }