Implement (most) review requests in

jenkinsci#9
jmozmoz · Oct 24, 2015 · d200a57 · d200a57
1 parent ecd0165
commit d200a57
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 27 deletions.
diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleJobProperty.java
@@ -31,7 +31,7 @@
 public class ThrottleJobProperty extends JobProperty<AbstractProject<?,?>> {
     // Moving category to categories, to support, well, multiple categories per job.
     @Deprecated transient String category;
-    
+
     private Integer maxConcurrentPerNode;
     private Integer maxConcurrentTotal;
     private List<String> categories;
@@ -47,7 +47,7 @@ public class ThrottleJobProperty extends JobProperty<AbstractProject<?,?>> {
      * functionality upgrades.
      */
     private Long configVersion;
-    
+
     @DataBoundConstructor
     public ThrottleJobProperty(Integer maxConcurrentPerNode,
                                Integer maxConcurrentTotal,
@@ -96,7 +96,7 @@ public Object readResolve() {
         }
 
         configVersion = 1L;
-        
+
         return this;
     }
 
@@ -121,33 +121,34 @@ public boolean getThrottleEnabled() {
         return throttleEnabled;
     }
 
-    public boolean getlimitOneJobWithMatchingParams() {
+    public boolean isLimitOneJobWithMatchingParams() {
         return limitOneJobWithMatchingParams;
     }
 
+
     public String getLimitOneJobByParams() {
         return limitOneJobByParams;
     }
 
     public String getThrottleOption() {
         return throttleOption;
     }
-    
+
     public List<String> getCategories() {
         return categories;
     }
-    
+
     public Integer getMaxConcurrentPerNode() {
         if (maxConcurrentPerNode == null)
             maxConcurrentPerNode = 0;
-        
+
         return maxConcurrentPerNode;
     }
 
     public Integer getMaxConcurrentTotal() {
         if (maxConcurrentTotal == null)
             maxConcurrentTotal = 0;
-        
+
         return maxConcurrentTotal;
     }
 
@@ -179,24 +180,24 @@ private static Item getItem(ItemGroup group, String name) {
             return group.getItem(name);
         }
     }
-    
+
     @Extension
     public static final class DescriptorImpl extends JobPropertyDescriptor {
         private List<ThrottleCategory> categories;
-        
+
         /** Map from category names, to properties including that category. */
         private Map<String,Map<ThrottleJobProperty,Void>> propertiesByCategory = new HashMap<String,Map<ThrottleJobProperty,Void>>();
-        
+
         public DescriptorImpl() {
             super(ThrottleJobProperty.class);
             load();
         }
-        
+
         @Override
         public String getDisplayName() {
             return "Throttle Concurrent Builds";
         }
-        
+
         @Override
         @SuppressWarnings("rawtypes")
         public boolean isApplicable(Class<? extends Job> jobType) {
@@ -236,10 +237,10 @@ public FormValidation doCheckMaxConcurrentTotal(@QueryParameter String value) {
             return checkNullOrInt(value);
         }
 
-        
+
         public ThrottleCategory getCategoryByName(String categoryName) {
             ThrottleCategory category = null;
-            
+
             for (ThrottleCategory tc : categories) {
                 if (tc.getCategoryName().equals(categoryName)) {
                     category = tc;
@@ -252,7 +253,7 @@ public ThrottleCategory getCategoryByName(String categoryName) {
         public void setCategories(List<ThrottleCategory> categories) {
             this.categories = categories;
         }
-        
+
         public List<ThrottleCategory> getCategories() {
             if (categories == null) {
                 categories = new ArrayList<ThrottleCategory>();
@@ -265,14 +266,14 @@ public ListBoxModel doFillCategoryItems() {
             ListBoxModel m = new ListBoxModel();
 
             m.add("(none)", "");
-            
+
             for (ThrottleCategory tc : getCategories()) {
                 m.add(tc.getCategoryName());
             }
 
             return m;
         }
-        
+
     }
 
     public static final class ThrottleCategory extends AbstractDescribableImpl<ThrottleCategory> {
@@ -292,18 +293,18 @@ public ThrottleCategory(String categoryName,
             this.nodeLabeledPairs =
                  nodeLabeledPairs == null ? new ArrayList<NodeLabeledPair>() : nodeLabeledPairs;
         }
-        
+
         public Integer getMaxConcurrentPerNode() {
             if (maxConcurrentPerNode == null)
                 maxConcurrentPerNode = 0;
-            
+
             return maxConcurrentPerNode;
         }
-        
+
         public Integer getMaxConcurrentTotal() {
             if (maxConcurrentTotal == null)
                 maxConcurrentTotal = 0;
-            
+
             return maxConcurrentTotal;
         }
 

diff --git a/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java b/src/main/java/hudson/plugins/throttleconcurrents/ThrottleQueueTaskDispatcher.java
@@ -94,7 +94,7 @@ else if (tjp.getThrottleOption().equals("category")) {
     public CauseOfBlockage canRun(Queue.Item item) {
         ThrottleJobProperty tjp = getThrottleJobProperty(item.task);
         if (tjp!=null && tjp.getThrottleEnabled()) {
-            if (tjp.getlimitOneJobWithMatchingParams() && isAnotherBuildWithSameParametersRunningOnAnyNode(item)) {
+            if (tjp.isLimitOneJobWithMatchingParams() && isAnotherBuildWithSameParametersRunningOnAnyNode(item)) {
                 LOGGER.info("A build with matching parameters is already running.");
                 return CauseOfBlockage.fromMessage(Messages._ThrottleQueueTaskDispatcher_OnlyOneWithMatchingParameters());
             }
@@ -189,13 +189,13 @@ private boolean isAnotherBuildWithSameParametersRunningOnNode(Node node, Queue.I
                     if (exec.getCurrentExecutable() != null &&
                         exec.getCurrentExecutable().getParent() != null &&
                         exec.getCurrentExecutable().getParent().getOwnerTask() != null &&
-                        exec.getCurrentExecutable().getParent().getOwnerTask().getName().equals(item.task.getName())) {
+                        exec.getCurrentExecutable().getParent().getOwnerTask().getName().equals(item.task.getDisplayName())) {
                         List<ParameterValue> executingUnitParams = getParametersFromWorkUnit(exec.getCurrentWorkUnit());
                         executingUnitParams = doFilterParams(paramsToCompare, executingUnitParams);
 
                         if (executingUnitParams.containsAll(itemParams)) {
-                            LOGGER.info("build (" + exec.getCurrentWorkUnit() + ") with identical parameters (" +
-                                        executingUnitParams + ") is already running.");
+                            LOGGER.debug("build (" + exec.getCurrentWorkUnit() + ") with identical parameters (" +
+                                         executingUnitParams + ") is already running.");
                             return true;
                         }
                     }
@@ -240,12 +240,16 @@ public List<ParameterValue> getParametersFromWorkUnit(WorkUnit unit) {
     }
 
     public List<ParameterValue> getParametersFromQueueItem(Queue.Item item) {
-        List<ParameterValue> paramsList = new ArrayList<ParameterValue>();
+        List<ParameterValue> paramsList;
 
         ParametersAction params = item.getAction(ParametersAction.class);
         if (params != null) {
             paramsList = params.getParameters();
         }
+        else
+        {
+        	paramsList  = new ArrayList<ParameterValue>();
+        }
         return paramsList;
     }