fixed JENKINS-12215

src/main/java/configurationslicing/executeshell/ExecuteShellSlicer.java

@@ -10,6 +10,7 @@
 import hudson.tasks.Shell;
 import hudson.util.DescribableList;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
@@ -88,42 +89,60 @@ private DescribableList<Builder,Descriptor<Builder>> getBuildersList(AbstractPro
 
         public boolean setValues(AbstractProject<?, ?> item, Set<String> set) {
             DescribableList<Builder,Descriptor<Builder>> buildersList = getBuildersList(item);
-            Shell shell = null;
             String command = set.iterator().next();
 
             // if the command is empty or NOTHING, remove the shell builder from the job
             if(command.equals(NOTHING) || command.equals("")) {
-                shell = (Shell)buildersList.get(Shell.class);
+                Shell shell = (Shell) buildersList.get(Shell.class);
                 if(shell != null) {
                     try {
+                    	// the remove command will persist the project
                         buildersList.remove(shell.getDescriptor());
                     } catch(java.io.IOException e) {
                         System.err.println("IOException Thrown removing shell value");
                         return false;
                     }
                 }
             } else {
-            	boolean replace = true;
+        		Shell newShell = new Shell(command);
             	// check to see if we need to replace the shell command.  This prevents persisting
             	// an empty change, which is important for keeping audit trails clean.
-            	Shell oldShell = buildersList.get(Shell.class);
-            	if (oldShell != null) {
-            		String oldCommand = oldShell.getCommand();
-            		if (command.equals(oldCommand)) {
-            			replace = false;
+            	Shell currentShell = buildersList.get(Shell.class);
+            	if (currentShell != null) {
+            		String oldCommand = currentShell.getCommand();
+            		if (!command.equals(oldCommand)) {
+            			return replaceBuilder(buildersList, currentShell, newShell);
             		}
-            	}
-            	if (replace) {
-	                shell = new Shell(command);
+            	} else {
 	                try {
-	                    buildersList.replace(shell);
+	                    buildersList.add(newShell);
 	                } catch(java.io.IOException e) {
-	                    System.err.println("IOException Thrown replacing shell value");
+	                    System.err.println("IOException Thrown add shell value");
 	                    return false;
 	                }
             	}
             }
-            return true;
+			return true;
+        }
+        /**
+         * If we do other builders, publishers, etc - this should be the pattern to use.
+         * @throws IOException 
+         */
+        private boolean replaceBuilder(DescribableList<Builder,Descriptor<Builder>> builders, Builder oldBuilder, Builder newBuilder) {
+        	List<Builder> newList = new ArrayList<Builder>(builders.toList());
+        	for (int i = 0; i < newList.size(); i++) {
+    			Builder b = newList.get(i);
+    			if (b == oldBuilder) {
+    				newList.set(i, newBuilder);
+    			}
+    		}
+        	try {
+        		builders.replaceBy(newList);
+        		return true;
+            } catch(java.io.IOException e) {
+            	System.err.println("IOException Thrown replacing builder list");
+            	return false;
+            }
         }
     }
 }