Skip to content

Commit

Permalink
[FIXED JENKINS-12748] differentiate between name and id
Browse files Browse the repository at this point in the history
  • Loading branch information
@imod
imod committed Feb 17, 2012
1 parent 4c99b77 commit 37223a7
Show file tree
Hide file tree
Showing 14 changed files with 126 additions and 89 deletions.
62 changes: 33 additions & 29 deletions src/main/java/org/jenkinsci/plugins/scriptler/ScriptlerManagment.java
View file
Expand Up @@ -205,8 +205,8 @@ public HttpResponse doDownloadScript(StaplerRequest req, StaplerResponse rsp, @Q

Parameter[] parameters = paramList.toArray(new Parameter[paramList.size()]);

final String finalName = saveScriptAndForward(info.getName(), info.getComment(), source, false, catalogName, id, parameters);
return new HttpRedirect("editScript?name=" + finalName);
final String finalName = saveScriptAndForward(id, info.getName(), info.getComment(), source, false, catalogName, id, parameters);
return new HttpRedirect("editScript?id=" + finalName);
}
}
final ForwardToView view = new ForwardToView(this, "catalog.jelly");
Expand All @@ -222,8 +222,10 @@ public HttpResponse doDownloadScript(StaplerRequest req, StaplerResponse rsp, @Q
* response
* @param rsp
* request
* @param id
* the script id (fileName)
* @param name
* the name for the file
* the name of the script
* @param comment
* a comment
* @param script
Expand All @@ -236,15 +238,15 @@ public HttpResponse doDownloadScript(StaplerRequest req, StaplerResponse rsp, @Q
* @throws IOException
* @throws ServletException
*/
public HttpResponse doScriptAdd(StaplerRequest req, StaplerResponse rsp, @QueryParameter("name") String name, @QueryParameter("comment") String comment,
@QueryParameter("script") String script, @QueryParameter("nonAdministerUsing") boolean nonAdministerUsing, String originCatalogName, String originId)
throws IOException, ServletException {
public HttpResponse doScriptAdd(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String id, @QueryParameter("name") String name,
@QueryParameter("comment") String comment, @QueryParameter("script") String script,
@QueryParameter("nonAdministerUsing") boolean nonAdministerUsing, String originCatalogName, String originId) throws IOException, ServletException {

checkPermission(Hudson.ADMINISTER);

Parameter[] parameters = extractParameters(req);

saveScriptAndForward(name, comment, script, nonAdministerUsing, originCatalogName, originId, parameters);
saveScriptAndForward(id, name, comment, script, nonAdministerUsing, originCatalogName, originId, parameters);
return new HttpRedirect("index");
}

Expand Down Expand Up @@ -278,35 +280,37 @@ private Parameter[] extractParameters(StaplerRequest req) throws ServletExceptio
/**
* Save the script details and return the forward to index
*
* @return the final name of the saved script
* @return the final name of the saved script - which is also the id of the script!
* @throws IOException
*/
private String saveScriptAndForward(String name, String comment, String script, boolean nonAdministerUsing, String originCatalogName, String originId,
Parameter[] parameters) throws IOException {
if (StringUtils.isEmpty(script) || StringUtils.isEmpty(name)) {
throw new IllegalArgumentException("'name' and 'script' must not be empty!");
private String saveScriptAndForward(String id, String name, String comment, String script, boolean nonAdministerUsing, String originCatalogName,
String originId, Parameter[] parameters) throws IOException {
script = script == null ? "TODO" : script;
if (StringUtils.isEmpty(id)) {
throw new IllegalArgumentException("'id' must not be empty!");
}

final String finalName = fixFileName(originCatalogName, name);
final String displayName = name == null ? id : name;
final String finalFileName = fixFileName(originCatalogName, id);

// save (overwrite) the file/script
File newScriptFile = new File(getScriptDirectory(), finalName);
File newScriptFile = new File(getScriptDirectory(), finalFileName);
Writer writer = new FileWriter(newScriptFile);
writer.write(script);
writer.close();

Script newScript = null;
if (!StringUtils.isEmpty(originId)) {
newScript = new Script(finalName, comment, true, originCatalogName, originId, new SimpleDateFormat("dd MMM yyyy HH:mm:ss a").format(new Date()),
parameters);
newScript = new Script(finalFileName, displayName, comment, true, originCatalogName, originId,
new SimpleDateFormat("dd MMM yyyy HH:mm:ss a").format(new Date()), parameters);
} else {
// save (overwrite) the meta information
newScript = new Script(finalName, comment, nonAdministerUsing, parameters);
newScript = new Script(finalFileName, displayName, comment, nonAdministerUsing, parameters);
}
ScriptlerConfiguration cfg = getConfiguration();
cfg.addOrReplace(newScript);
cfg.save();
return finalName;
return finalFileName;
}

/**
Expand All @@ -321,16 +325,16 @@ private String saveScriptAndForward(String name, String comment, String script,
* @return forward to 'index'
* @throws IOException
*/
public HttpResponse doRemoveScript(StaplerRequest res, StaplerResponse rsp, @QueryParameter("name") String name) throws IOException {
public HttpResponse doRemoveScript(StaplerRequest res, StaplerResponse rsp, @QueryParameter("id") String id) throws IOException {
checkPermission(Hudson.ADMINISTER);

// remove the file
File oldScript = new File(getScriptDirectory(), name);
File oldScript = new File(getScriptDirectory(), id);
oldScript.delete();

// remove the meta information
ScriptlerConfiguration cfg = getConfiguration();
cfg.removeScript(name);
cfg.removeScript(id);
cfg.save();

return new HttpRedirect("index");
Expand Down Expand Up @@ -389,8 +393,8 @@ public HttpResponse doUploadScript(StaplerRequest req) throws IOException, Servl
* @throws IOException
* @throws ServletException
*/
public void doRunScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("name") String scriptName) throws IOException, ServletException {
Script script = ScriptHelper.getScript(scriptName, true);
public void doRunScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String id) throws IOException, ServletException {
Script script = ScriptHelper.getScript(id, true);
checkPermission(getRequiredPermissionForRunScript());

final boolean isAdmin = Jenkins.getInstance().getACL().hasPermission(Jenkins.ADMINISTER);
Expand Down Expand Up @@ -421,8 +425,8 @@ public void doRunScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter
* @throws IOException
* @throws ServletException
*/
public void doTriggerScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("scriptName") String scriptName,
@QueryParameter("script") String scriptSrc, @QueryParameter("node") String node) throws IOException, ServletException {
public void doTriggerScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String id, @QueryParameter("script") String scriptSrc,
@QueryParameter("node") String node) throws IOException, ServletException {

checkPermission(getRequiredPermissionForRunScript());

Expand All @@ -433,13 +437,13 @@ public void doTriggerScript(StaplerRequest req, StaplerResponse rsp, @QueryParam
final boolean isChangeScriptAllowed = isAdmin || allowRunScriptEdit();

if (!isChangeScriptAllowed) {
tempScript = ScriptHelper.getScript(scriptName, true);
tempScript = ScriptHelper.getScript(id, true);
// use original script, user has no permission to change it!s
scriptSrc = tempScript.script;
} else {
// set the script info back to the request, to display it together with
// the output.
tempScript = ScriptHelper.getScript(scriptName, false);
tempScript = ScriptHelper.getScript(id, false);
tempScript.setScript(scriptSrc);
}

Expand Down Expand Up @@ -485,10 +489,10 @@ private String[] resolveSlaveNames(String nameAlias) {
* @throws IOException
* @throws ServletException
*/
public void doEditScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("name") String scriptName) throws IOException, ServletException {
public void doEditScript(StaplerRequest req, StaplerResponse rsp, @QueryParameter("id") String id) throws IOException, ServletException {
checkPermission(Hudson.ADMINISTER);

Script script = ScriptHelper.getScript(scriptName, true);
Script script = ScriptHelper.getScript(id, true);
req.setAttribute("script", script);
req.getView(this, "edit.jelly").forward(req, rsp);
}
Expand Down
8 changes: 4 additions & 4 deletions src/main/java/org/jenkinsci/plugins/scriptler/ScritplerPluginImpl.java
View file
Expand Up @@ -80,17 +80,17 @@ private void synchronizeConfig() throws IOException {
// check if all physical files are available in the configuration
// if not, add it to the configuration
for (File file : availablePhysicalScripts) {
if (cfg.getScriptByName(file.getName()) == null) {
cfg.addOrReplace(new Script(file.getName(), Messages.script_loaded_from_directory(), false, null));
if (cfg.getScriptById(file.getName()) == null) {
cfg.addOrReplace(new Script(file.getName(), file.getName(), Messages.script_loaded_from_directory(), false, null));
}
}

// check if all scripts in the configuration are physically available
// if not, mark it as missing
Set<Script> unavailableScripts = new HashSet<Script>();
for (Script s : cfg.getScripts()) {
if (!(new File(scriptDirectory, s.name).exists())) {
unavailableScripts.add(new Script(s.name, s.comment, false, false));
if (!(new File(scriptDirectory, s.getId()).exists())) {
unavailableScripts.add(new Script(s.getId(), s.comment, false, false));
}
}
for (Script script : unavailableScripts) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/org/jenkinsci/plugins/scriptler/config/NamedResource.java
View file
Expand Up @@ -3,5 +3,6 @@
public interface NamedResource {

public String getName();
public String getId();

}
64 changes: 44 additions & 20 deletions src/main/java/org/jenkinsci/plugins/scriptler/config/Script.java
View file
Expand Up @@ -27,6 +27,7 @@

public class Script implements Comparable<Script>, NamedResource {

private String id;
public final String name;
public final String comment;
public final boolean available;
Expand All @@ -47,7 +48,8 @@ public class Script implements Comparable<Script>, NamedResource {
* used to create/update a new script in the UI
*/
@DataBoundConstructor
public Script(String name, String comment, boolean nonAdministerUsing, Parameter[] parameters) {
public Script(String id, String name, String comment, boolean nonAdministerUsing, Parameter[] parameters) {
this.id = id;
this.name = name;
this.comment = comment;
this.available = true;
Expand All @@ -61,8 +63,9 @@ public Script(String name, String comment, boolean nonAdministerUsing, Parameter
/**
* used during upload of a new script
*/
public Script(String name, String comment) {
this.name = name;
public Script(String id, String comment) {
this.id = id;
this.name = id;
this.comment = comment;
this.available = true;
this.originCatalog = null;
Expand All @@ -75,8 +78,9 @@ public Script(String name, String comment) {
/**
* used during plugin start to synchronize available scripts
*/
public Script(String name, String comment, boolean available, boolean nonAdministerUsing) {
this.name = name;
public Script(String id, String comment, boolean available, boolean nonAdministerUsing) {
this.id = id;
this.name = id;
this.comment = comment;
this.available = available;
this.originCatalog = null;
Expand All @@ -89,7 +93,9 @@ public Script(String name, String comment, boolean available, boolean nonAdminis
/**
* Constructor to create a script imported from a foreign catalog.
*/
public Script(String name, String comment, boolean available, String originCatalog, String originScript, String originDate, Parameter[] parameters) {
public Script(String id, String name, String comment, boolean available, String originCatalog, String originScript, String originDate,
Parameter[] parameters) {
this.id = id;
this.name = name;
this.comment = comment;
this.available = available;
Expand All @@ -104,8 +110,9 @@ public Script(String name, String comment, boolean available, String originCatal
/**
* used to merge scripts
*/
public Script(String name, String comment, boolean available, String originCatalog, String originScript, String originDate, boolean nonAdministerUsing,
Parameter[] parameters) {
public Script(String id, String name, String comment, boolean available, String originCatalog, String originScript, String originDate,
boolean nonAdministerUsing, Parameter[] parameters) {
this.id = id;
this.name = name;
this.comment = comment;
this.available = available;
Expand Down Expand Up @@ -147,25 +154,42 @@ public void setNonAdministerUsing(boolean nonAdministerUsing) {
* @see java.lang.Comparable#compareTo(java.lang.Object)
*/
public int compareTo(Script o) {
return name.compareTo(o.name);
return id.compareTo(o.id);
}

/*
* (non-Javadoc)
*
/**
* Previously we used not to have an id, but only a name.
*/
public Object readResolve() {
if (id == null) {
id = name;
}
return this;
}

/**
* @return the id
*/
public String getId() {
return id;
}

public String toString() {
return "[Script: " + id + ":" + name + "]";
}

/**
* @see java.lang.Object#hashCode()
*/
@Override
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((name == null) ? 0 : name.hashCode());
result = prime * result + ((id == null) ? 0 : id.hashCode());
return result;
}

/*
* (non-Javadoc)
*
/**
* @see java.lang.Object#equals(java.lang.Object)
*/
@Override
Expand All @@ -176,15 +200,15 @@ public boolean equals(Object obj) {
if (obj == null) {
return false;
}
if (!(obj instanceof Script)) {
if (getClass() != obj.getClass()) {
return false;
}
Script other = (Script) obj;
if (name == null) {
if (other.name != null) {
if (id == null) {
if (other.id != null) {
return false;
}
} else if (!name.equals(other.name)) {
} else if (!id.equals(other.id)) {
return false;
}
return true;
Expand Down
17 changes: 9 additions & 8 deletions src/main/java/org/jenkinsci/plugins/scriptler/config/ScriptSet.java
View file
Expand Up @@ -30,32 +30,32 @@
import org.apache.commons.lang.StringUtils;

/**
* @author domi
* @author imod
*
*/
public class ScriptSet {

// have it sorted
protected Set<Script> scriptSet = new TreeSet<Script>();

public Script getScriptByName(String name) {
public Script getScriptById(String id) {
for (Script scr : scriptSet) {
if (scr.name.equals(name)) {
if (scr.getId().equals(id)) {
return scr;
}
}
return null;
}

public void removeScript(String name) {
Script s = getScriptByName(name);
public void removeScript(String id) {
Script s = getScriptById(id);
scriptSet.remove(s);
}

public void addOrReplace(Script script) {
if (script != null) {
if (scriptSet.contains(script)) {
Script oldScript = this.getScriptByName(script.name);
Script oldScript = this.getScriptById(script.getId());
if (oldScript != null) {
Script mergedScript = merge(oldScript, script);
scriptSet.remove(script);
scriptSet.add(mergedScript);
Expand All @@ -66,11 +66,12 @@ public void addOrReplace(Script script) {
}

private Script merge(Script origin, Script newScript) {
String name = StringUtils.isEmpty(newScript.name) ? origin.name : newScript.name;
String comment = StringUtils.isEmpty(newScript.comment) ? origin.comment : newScript.comment;
String originCatalog = StringUtils.isEmpty(newScript.originCatalog) ? origin.originCatalog : newScript.originCatalog;
String originScript = StringUtils.isEmpty(newScript.originScript) ? origin.originScript : newScript.originScript;
String originDate = StringUtils.isEmpty(newScript.originDate) ? origin.originDate : newScript.originDate;
return new Script(origin.getName(), comment, newScript.available, originCatalog, originScript, originDate, newScript.nonAdministerUsing,
return new Script(newScript.getId(), name, comment, newScript.available, originCatalog, originScript, originDate, newScript.nonAdministerUsing,
newScript.getParameters());
}

Expand Down

0 comments on commit 37223a7

Please sign in to comment.