- deserialize the parameters correctly

- in case the script was not found, put also a comment in the code in run mode (and avoid NPE)
jenkinsci · Feb 13, 2018 · 9f24e9e · 9f24e9e
1 parent b7316e4
commit 9f24e9e
Show file tree

Hide file tree

Showing 6 changed files with 106 additions and 64 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerManagement.java b/src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerManagement.java
@@ -429,16 +429,20 @@ public void doRunScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter
             //TODO check if we cannot do better here
             throw new IOException(Messages.scriptNotFound(id));
         }
-
-        boolean canByPassScriptApproval = Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS);
-
-        // we do not want user with approval right to auto-approve script when landing on that page
-        if(!ScriptHelper.isApproved(script.script, false)){
-            req.setAttribute("notApprovedYet", true);
-        }
+        if(script.script == null){
+            req.setAttribute("scriptNotFound", true);
+        }else{
+            boolean canByPassScriptApproval = Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS);
+
+            // we do not want user with approval right to auto-approve script when landing on that page
+            if(!ScriptHelper.isApproved(script.script, false)){
+                req.setAttribute("notApprovedYet", true);
+            }
 
+            req.setAttribute("canByPassScriptApproval", canByPassScriptApproval);
+        }
+
         req.setAttribute("script", script);
-        req.setAttribute("canByPassScriptApproval", canByPassScriptApproval);
         // set default selection
         req.setAttribute("currentNode", MASTER);
         req.getView(this, "runScript.jelly").forward(req, rsp);
@@ -480,7 +484,7 @@ public void doTriggerScript(StaplerRequest req, StaplerResponse rsp, @QueryParam
         String originalScriptSourceCode = originalScript.script;
 
         Script tempScript = originalScript.copy();
-        if(originalScriptSourceCode.equals(scriptSrc)){
+        if(originalScriptSourceCode != null && originalScriptSourceCode.equals(scriptSrc)){
             // not copied by default
             tempScript.setScript(originalScriptSourceCode);
         }else{
@@ -638,8 +642,21 @@ public void doShowScript(StaplerRequest req, StaplerResponse rsp, @QueryParamete
      */
     public void doEditScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String id) throws IOException, ServletException {
         checkPermission(ScriptlerPluginImpl.CONFIGURE);
-
+    
         Script script = ScriptHelper.getScript(id, true);
+        if(script.script == null){
+            req.setAttribute("scriptNotFound", true);
+        }else{
+            boolean canByPassScriptApproval = Jenkins.getInstance().hasPermission(Jenkins.RUN_SCRIPTS);
+
+            // we do not want user with approval right to auto-approve script when landing on that page
+            if(!ScriptHelper.isApproved(script.script, false)){
+                req.setAttribute("notApprovedYet", true);
+            }
+
+            req.setAttribute("canByPassScriptApproval", canByPassScriptApproval);
+        }
+
         req.setAttribute("script", script);
         req.getView(this, "edit.jelly").forward(req, rsp);
     }

diff --git a/src/main/java/org/jenkinsci/plugins/scriptler/SyncUtil.java b/src/main/java/org/jenkinsci/plugins/scriptler/SyncUtil.java
@@ -63,15 +63,11 @@ public static void syncDirWithCfg(File scriptDirectory, ScriptlerConfiguration c
             if ((new File(scriptDirectory, s.getScriptPath()).exists())) {
                 s.setAvailable(true);
             } else {
-                unavailableScripts.add(new Script(s.getId(), s.comment, false, false, false));
+                Script unavailableScript = new Script(s.getId(), s.comment, false, false, false);
+                // to no loose parameter configuration if we loose the file
+                unavailableScript.setParameters(s.getParameters());
+                unavailableScripts.add(unavailableScript);
                 LOGGER.info("for repo '" + scriptDirectory.getAbsolutePath() + "' " + s + " is not available!");
-
-
-//                Script unavailableScript = new Script(s.getId(), s.getName(), s.comment, false, s.getParameters(), false);
-//                unavailableScript.setAvailable(false);
-//    
-//                unavailableScripts.add(unavailableScript);
-//                LOGGER.info("for repo '" + scriptDirectory.getAbsolutePath() + "' " + s + " is not available!");
             }
         }
 

diff --git a/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/edit.jelly b/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/edit.jelly
@@ -36,47 +36,64 @@ THE SOFTWARE.
 				<j:out value="${%title}" />
 			</h1>
 			<f:form method="post" action="scriptAdd">
-				<j:jelly xmlns:j="jelly:core">
-				    <f:entry title="${%Id}" description="${%IdDescription}">
-                        <f:textbox name="id" value="${script.id}" />
-                    </f:entry>
-					<f:entry title="${%Name}">
-						<f:textbox name="name" value="${script.name}" />
-					</f:entry>
-					<f:entry title="${%Comment}">
-						<f:textbox name="comment" value="${script.comment}" />
-					</f:entry>
-                    <f:entry title="${%Permission}" description="${%PermissionDescription}">
-						<f:checkbox name="nonAdministerUsing" checked="${script.nonAdministerUsing}" />
-					</f:entry>
-                    <f:entry title="${%Restriction}" description="${%RestrictionDescription}">
-                        <f:checkbox name="onlyMaster" checked="${script.onlyMaster}" />
-		            </f:entry>
-                    <f:block>
-	                    <table>
-	                        <f:optionalBlock name="defineParams" title="${%ParametersDescription}" checked="${!empty(script.parameters)}">
-	                            <f:entry title="${%Parameters}" field="parameters">
-	                                <f:repeatable var="param" items="${script.parameters}" name="parameters" noAddButton="true" minimum="1">
-	                                    <table width="100%">
-	                                        <f:entry>
-	                                            ${%ParameterName} <input type="text" name="name" value="${param.name}" size="50"/>
-	                                            ${%ParameterDefaultValue} <input type="text" name="value" value="${param.value}" size="80"/>
-	                                            <input type="button" name="delete_button" value="${%DeleteParameter}" class="repeatable-delete show-if-not-only" style="margin-left: 1em;" />
-	                                            <input type="button" name="add_button" value="${%AddParameter}" class="repeatable-add show-if-last" />
-	                                        </f:entry>
-	                                    </table>
-	                                </f:repeatable>
-	                            </f:entry>
-	                        </f:optionalBlock>
-	                    </table>
-	                </f:block>					
-					<f:entry title="${%Script}">
-						<textarea id="script" name="script" class="script">
-							<j:out value="${script.script}" />
-						</textarea>
-					</f:entry>
-				</j:jelly>
+				<f:entry title="${%Id}" description="${%IdDescription}">
+					<f:textbox name="id" value="${script.id}" />
+				</f:entry>
+				<f:entry title="${%Name}">
+					<f:textbox name="name" value="${script.name}" />
+				</f:entry>
+				<f:entry title="${%Comment}">
+					<f:textbox name="comment" value="${script.comment}" />
+				</f:entry>
+				<f:entry title="${%Permission}" description="${%PermissionDescription}">
+					<f:checkbox name="nonAdministerUsing" checked="${script.nonAdministerUsing}" />
+				</f:entry>
+				<f:entry title="${%Restriction}" description="${%RestrictionDescription}">
+					<f:checkbox name="onlyMaster" checked="${script.onlyMaster}" />
+				</f:entry>
 				<f:block>
+					<table>
+						<f:optionalBlock name="defineParams" title="${%ParametersDescription}" checked="${!empty(script.parameters)}">
+							<f:entry title="${%Parameters}" field="parameters">
+								<f:repeatable var="param" items="${script.parameters}" name="parameters" noAddButton="true" minimum="1">
+									<table width="100%">
+										<f:entry>
+											${%ParameterName} <input type="text" name="name" value="${param.name}" size="50"/>
+											${%ParameterDefaultValue} <input type="text" name="value" value="${param.value}" size="80"/>
+											<input type="button" name="delete_button" value="${%DeleteParameter}" class="repeatable-delete show-if-not-only" style="margin-left: 1em;" />
+											<input type="button" name="add_button" value="${%AddParameter}" class="repeatable-add show-if-last" />
+										</f:entry>
+									</table>
+								</f:repeatable>
+							</f:entry>
+						</f:optionalBlock>
+					</table>
+				</f:block>					
+				<f:entry title="${%Script}">
+					<textarea id="script" name="script" class="script">
+						<j:choose>
+							<j:when test="${scriptNotFound}">
+								${%ScriptNotFound}
+							</j:when>
+							<j:otherwise>
+								<j:out value="${script.script}" />
+							</j:otherwise>
+						</j:choose>
+					</textarea>
+				</f:entry>
+				<f:block>
+					<j:if test="${notApprovedYet}">
+						<j:choose>
+							<j:when test="${h.hasPermission(it, app.RUN_SCRIPTS)}">
+								<div class="warning">${%NotApprovedYetButHasRightWarn}</div>
+								<div class="info">${%NotApprovedYetButHasRightInfo}</div>
+							</j:when>
+							<j:otherwise>
+								<div class="warning">${%NotApprovedYet}</div>
+							</j:otherwise>
+						</j:choose>			
+						<br />
+					</j:if>
 					<f:submit value="${%Submit}" />
 				</f:block>
 			</f:form>

diff --git a/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/edit.properties b/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/edit.properties
@@ -36,3 +36,7 @@ DeleteParameter=Delete
 AddParameter=Add Parameter
 Script=Script
 Submit=Submit
+ScriptNotFound=// Script source file was not found
+NotApprovedYet=Script not yet approved, consider asking your administrator to approve it.
+NotApprovedYetButHasRightWarn=Script not yet approved
+NotApprovedYetButHasRightInfo=By saving it you will automatically approve it.
diff --git a/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/runScript.jelly b/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/runScript.jelly
@@ -101,13 +101,15 @@ THE SOFTWARE.
 				</table>
 
 				<textarea id="script" name="script" class="script">
-					${script.script}
+					<j:choose>
+						<j:when test="${scriptNotFound}">
+							${%ScriptNotFound}
+						</j:when>
+						<j:otherwise>
+							<j:out value="${script.script}" />
+						</j:otherwise>
+					</j:choose>
 				</textarea>
-				<div align="right">
-					<f:submit value="${%Run}" name="run" />
-				</div>
-				<script>$('script').focus();</script>
-			</form>
 			<j:if test="${notApprovedYet}">
 				<j:choose>
 					<j:when test="${h.hasPermission(it, app.RUN_SCRIPTS)}">
@@ -119,6 +121,11 @@ THE SOFTWARE.
 					</j:otherwise>
 				</j:choose>
 			</j:if>
+				<div align="right">
+					<f:submit value="${%Run}" name="run" />
+				</div>
+				<script>$('script').focus();</script>
+			</form>
 			<j:if test="${output!=null}">
 				<h2>${%Result}</h2>
 				<pre>

diff --git a/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/runScript.properties b/src/main/resources/org/jenkinsci/plugins/scriptler/ScriptlerManagement/runScript.properties
@@ -32,6 +32,7 @@ ParameterName=Name:
 ParameterValue=Value:
 Run=Run
 Result=Result
+ScriptNotFound=// Script source file was not found
 NotApprovedYet=Script not yet approved, consider asking your administrator to approve it.
 NotApprovedYetButHasRightWarn=Script not yet approved
 NotApprovedYetButHasRightInfo=By running it you will automatically approve it.