Add user defined properties to lockable resource (#394)

* Added UI to set properties on Lockable Resources * Document properties in the README.md * Expose the properties to environment variables * Removed unnecessary Copyright to 6WIND * Removed unnecessary Copyright to 6WIND * maintain lock ordering when reporting in environment variables Fixes #300 : The order of the locked resources in the environment variable does not match the order in the extra parameter * maintain lock ordering when reporting in environment variables Fixes #300 : The order of the locked resources in the environment variable does not match the order in the extra parameter * maintain lock ordering when reporting in environment variables Fixes #300 : The order of the locked resources in the environment variable does not match the order in the extra parameter * maintain lock ordering when reporting in environment variables Fixes #300 : The order of the locked resources in the environment variable does not match the order in the extra parameter * Revert "maintain lock ordering when reporting in environment variables" This reverts commit be88b85. * Revert "maintain lock ordering when reporting in environment variables" This reverts commit 82a7770. * Revert "maintain lock ordering when reporting in environment variables" This reverts commit 1653c7b. * Revert "maintain lock ordering when reporting in environment variables" This reverts commit dfdfc5b. * Adding properties in a colum of the lock descriptions * and now fix it after master merge * style: Remove commented out line --------- Co-authored-by: Gaspard Petit <[email protected]> Co-authored-by: Jan-Frederik Schmidt <[email protected]>
jenkinsci · May 2, 2023 · 9fa89cf · 9fa89cf
1 parent 5cbaf22
commit 9fa89cf
Show file tree

Hide file tree

Showing 14 changed files with 284 additions and 69 deletions.
diff --git a/README.md b/README.md
@@ -100,11 +100,12 @@ lock(resource: 'staging-server', inversePrecedence: true) {
 }
 ```
 
-#### Resolve a variable configured with the resource name
+#### Resolve a variable configured with the resource name and properties 
 
 ```groovy
 lock(label: 'some_resource', variable: 'LOCKED_RESOURCE') {
   echo env.LOCKED_RESOURCE
+  echo env.LOCKED_RESOURCE_PROP_ABC
 }
 ```
 
@@ -117,9 +118,11 @@ lock(label: 'some_resource', variable: 'LOCKED_RESOURCE', quantity: 2) {
 
   // first lock
   echo env.LOCKED_RESOURCE0
+  echo env.LOCKED_RESOURCE0_PROP_ABC
 
   // second lock
   echo env.LOCKED_RESOURCE1
+  echo env.LOCKED_RESOURCE1_PROP_ABC
 }
 ```
 
@@ -220,6 +223,10 @@ unclassified:
         reservedBy: "Reserved due maintenance window"
       - name: "S7_1200_2"
         labels: "plc:S7 model:1200"
+      - name: "Resource-with-properties"
+        properties:
+          - name: "Property-A"
+            value: "Value"
 ```
 
 Properties *description*, *labels* and *reservedBy* are optional.

diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java b/src/main/java/org/jenkins/plugins/lockableresources/LockStepExecution.java
@@ -9,8 +9,9 @@
 import java.io.PrintStream;
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
+import java.util.Map.Entry;
 import java.util.Locale;
 import java.util.Map;
 import java.util.logging.Level;
@@ -111,7 +112,7 @@ public boolean start() throws Exception {
 
   @SuppressFBWarnings(value = "REC_CATCH_EXCEPTION", justification = "not sure which exceptions might be catch.")
   public static void proceed(
-    final List<String> resourceNames,
+    final LinkedHashMap<String, List<LockableResourceProperty>> lockedResources,
     StepContext context,
     String resourceDescription,
     final String variable,
@@ -136,7 +137,7 @@ public static void proceed(
       BodyInvoker bodyInvoker =
         context
           .newBodyInvoker()
-          .withCallback(new Callback(resourceNames, resourceDescription, inversePrecedence));
+          .withCallback(new Callback(new ArrayList<>(lockedResources.keySet()), resourceDescription, inversePrecedence));
       if (variable != null && variable.length() > 0) {
         // set the variable for the duration of the block
         bodyInvoker.withContext(
@@ -147,11 +148,18 @@ public static void proceed(
 
               @Override
               public void expand(@NonNull EnvVars env) {
-                final Map<String, String> variables = new HashMap<>();
-                final String resources = String.join(",", resourceNames);
-                variables.put(variable, resources);
-                for (int index = 0; index < resourceNames.size(); ++index) {
-                  variables.put(variable + index, resourceNames.get(index));
+                final LinkedHashMap<String, String> variables = new LinkedHashMap<>();
+                final String resourceNames = lockedResources.keySet().stream().collect(Collectors.joining(","));
+                variables.put(variable, resourceNames);
+                int index = 0;
+                for (Entry<String, List<LockableResourceProperty>> lockResourceEntry : lockedResources.entrySet()) {
+                  String lockEnvName = variable + index;
+                  variables.put(lockEnvName, lockResourceEntry.getKey());
+                  for (LockableResourceProperty lockProperty : lockResourceEntry.getValue()) {
+                    String propEnvName = lockEnvName + "_" + lockProperty.getName();
+                    variables.put(propEnvName, lockProperty.getValue());
+                  }
+                  ++index;
                 }
                 LOGGER.finest("Setting "
                   + variables.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(", "))

diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResource.java
@@ -90,6 +90,7 @@ public class LockableResource extends AbstractDescribableImpl<LockableResource>
    * being held, it becomes ephemeral and will disappear when freed.
    */
   private boolean ephemeral;
+  private List<LockableResourceProperty> properties = new ArrayList<>();
 
   private long queueItemId = NOT_QUEUED;
   private String queueItemProject = null;
@@ -279,6 +280,16 @@ private boolean labelsContain(String candidate) {
     return this.getLabelsAsList().contains(candidate);
   }
 
+  @Exported
+  public List<LockableResourceProperty> getProperties() {
+    return properties;
+  }
+
+  @DataBoundSetter
+  public void setProperties(List<LockableResourceProperty> properties) {
+    this.properties = (properties == null ? new ArrayList<>() : properties);
+  }
+
   /**
    * Checks if the script matches the requirement.
    *

diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResourceProperty.java
@@ -0,0 +1,58 @@
+package org.jenkins.plugins.lockableresources;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
+import hudson.model.AbstractDescribableImpl;
+import hudson.model.Descriptor;
+import java.io.Serializable;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
+import org.kohsuke.stapler.export.Exported;
+
+public class LockableResourceProperty extends AbstractDescribableImpl<LockableResourceProperty>
+  implements Serializable {
+
+  private String name;
+  private String value;
+
+  @DataBoundConstructor
+  public LockableResourceProperty() {
+  }
+
+  @DataBoundSetter
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  @DataBoundSetter
+  public void setValue(String value) {
+    this.value = value;
+  }
+
+  @Exported
+  public String getName() {
+    return name;
+  }
+
+  @Exported
+  public String getValue() {
+    return value;
+  }
+
+  @Override
+  public String toString() {
+    return name;
+  }
+
+  @Extension
+  public static class DescriptorImpl extends Descriptor<LockableResourceProperty> {
+
+    @NonNull
+    @Override
+    public String getDisplayName() {
+      return "Property";
+    }
+  }
+
+  private static final long serialVersionUID = 1L;
+}
diff --git a/src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java b/src/main/java/org/jenkins/plugins/lockableresources/LockableResourcesManager.java
@@ -26,14 +26,15 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.LinkedHashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.stream.Collectors;
 import jenkins.model.GlobalConfiguration;
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
@@ -457,10 +458,10 @@ public synchronized boolean lock(
       }
       if (context != null) {
         // since LockableResource contains transient variables, they cannot be correctly serialized
-        // hence we use their unique resource names
-        List<String> resourceNames = new ArrayList<>();
+        // hence we use their unique resource names and properties
+        LinkedHashMap<String, List<LockableResourceProperty>> resourceNames = new LinkedHashMap<>();
         for (LockableResource resource : resources) {
-          resourceNames.add(resource.getName());
+          resourceNames.put(resource.getName(), resource.getProperties());
         }
         LockStepExecution.proceed(resourceNames, context, logmessage, variable, inversePrecedence);
       }
@@ -571,13 +572,13 @@ public synchronized void unlockNames(
         // remove context from queue and process it
         unqueueContext(nextContext.getContext());
 
-        List<String> resourceNamesToLock = new ArrayList<>();
+        LinkedHashMap<String, List<LockableResourceProperty>> resourcesToLock = new LinkedHashMap<>();
 
         // lock all (old and new resources)
         for (LockableResource requiredResource : requiredResourceForNextContext) {
           try {
             requiredResource.setBuild(nextContext.getContext().get(Run.class));
-            resourceNamesToLock.add(requiredResource.getName());
+            resourcesToLock.put(requiredResource.getName(), requiredResource.getProperties());
           } catch (Exception e) {
             // skip this context, as the build cannot be retrieved (maybe it was deleted while
             // running?)
@@ -616,7 +617,7 @@ public synchronized void unlockNames(
 
         // continue with next context
         LockStepExecution.proceed(
-          resourceNamesToLock,
+          resourcesToLock,
           nextContext.getContext(),
           nextContext.getResourceDescription(),
           nextContext.getVariableName(),
@@ -743,6 +744,28 @@ public synchronized boolean createResourceWithLabel(String name, String label) {
     return false;
   }
 
+  @Restricted(NoExternalUse.class)
+  public synchronized boolean createResourceWithLabelAndProperties(String name, String label, Map<String, String> properties) {
+    if (name != null && label != null && properties != null) {
+      LockableResource existent = fromName(name);
+      if (existent == null) {
+        LockableResource resource = new LockableResource(name);
+        resource.setLabels(label);
+        resource.setProperties(
+          properties.entrySet().stream().map(e -> {
+            LockableResourceProperty p = new LockableResourceProperty();
+            p.setName(e.getKey());
+            p.setValue(e.getValue());
+            return p;
+          }).collect(Collectors.toList()));
+        getResources().add(resource);
+        save();
+        return true;
+      }
+    }
+    return false;
+  }
+
   /**
    * Reserves an available resource for the userName indefinitely (until that person, or some
    * explicit scripted action, decides to release the resource).
@@ -873,13 +896,13 @@ public synchronized void unreserve(List<LockableResource> resources) {
       return;
     } else {
       unreserveResources(resources);
-      List<String> resourceNamesToLock = new ArrayList<>();
+      LinkedHashMap<String, List<LockableResourceProperty>> resourcesToLock = new LinkedHashMap<>();
 
       // lock all (old and new resources)
       for (LockableResource requiredResource : requiredResourceForNextContext) {
         try {
           requiredResource.setBuild(nextContext.getContext().get(Run.class));
-          resourceNamesToLock.add(requiredResource.getName());
+          resourcesToLock.put(requiredResource.getName(), requiredResource.getProperties());
         } catch (Exception e) {
           // skip this context, as the build cannot be retrieved (maybe it was deleted while
           // running?)
@@ -897,7 +920,7 @@ public synchronized void unreserve(List<LockableResource> resources) {
 
       // continue with next context
       LockStepExecution.proceed(
-        resourceNamesToLock,
+        resourcesToLock,
         nextContext.getContext(),
         nextContext.getResourceDescription(),
         nextContext.getVariableName(),
@@ -1153,7 +1176,7 @@ public synchronized List<LockableResource> checkResourcesAvailability(
     }
 
     // Find remaining resources
-    LinkedHashSet<LockableResource> allSelected = new LinkedHashSet<>();
+    List<LockableResource> allSelected = new ArrayList<>();
 
     for (LockableResourcesCandidatesStruct requiredResources : requiredResourcesCandidatesList) {
       List<LockableResource> candidates = requiredResources.candidates;
@@ -1202,7 +1225,7 @@ public synchronized List<LockableResource> checkResourcesAvailability(
       allSelected.addAll(selected);
     }
 
-    return new ArrayList<>(allSelected);
+    return allSelected;
   }
 
   /*