opendev/gerrit
Code Issues Proposed changes

Don't pass null arguments to hooks

Browse Source 
If the change URL isn't available (e.g. because we don't have
gerrit.canonicalWebUrl configured) we cannot pass down the
--change-url flag to certain hooks.  Safeguard the code from
that failure case.

Bug: issue 641
Change-Id: Ib26ec8759e0090aadfc815c17fe7db0f299c640e
Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2010-08-19 12:33:07 -07:00
parent 135db6e96f
commit a5f868563f
1 changed files with 34 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

88
gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java
View File

@@ -172,6 +172,13 @@ public class ChangeHookRunner {
}
}
private void addArg(List<String> args, String name, String value) {
if (value != null) {
args.add(name);
args.add(value);
}
}
/**
* Fire the Patchset Created Hook.
*
@@ -190,20 +197,13 @@ public class ChangeHookRunner {
final List<String> args = new ArrayList<String>();
args.add(patchsetCreatedHook.getAbsolutePath());
args.add("--change");
args.add(event.change.id);
args.add("--change-url");
args.add(event.change.url);
args.add("--project");
args.add(event.change.project);
args.add("--branch");
args.add(event.change.branch);
args.add("--uploader");
args.add(getDisplayName(uploader.getAccount()));
args.add("--commit");
args.add(event.patchSet.revision);
args.add("--patchset");
args.add(event.patchSet.number);
addArg(args, "--change", event.change.id);
addArg(args, "--change-url", event.change.url);
addArg(args, "--project", event.change.project);
addArg(args, "--branch", event.change.branch);
addArg(args, "--uploader", getDisplayName(uploader.getAccount()));
addArg(args, "--commit", event.patchSet.revision);
addArg(args, "--patchset", event.patchSet.number);
runHook(getRepo(change), args);
}
@@ -238,23 +238,15 @@ public class ChangeHookRunner {
final List<String> args = new ArrayList<String>();
args.add(commentAddedHook.getAbsolutePath());
args.add("--change");
args.add(event.change.id);
args.add("--change-url");
args.add(event.change.url);
args.add("--project");
args.add(event.change.project);
args.add("--branch");
args.add(event.change.branch);
args.add("--author");
args.add(getDisplayName(account));
args.add("--commit");
args.add(event.patchSet.revision);
args.add("--comment");
args.add(comment == null ? "" : comment);
addArg(args, "--change", event.change.id);
addArg(args, "--change-url", event.change.url);
addArg(args, "--project", event.change.project);
addArg(args, "--branch", event.change.branch);
addArg(args, "--author", getDisplayName(account));
addArg(args, "--commit", event.patchSet.revision);
addArg(args, "--comment", comment == null ? "" : comment);
for (Map.Entry<ApprovalCategory.Id, ApprovalCategoryValue.Id> approval : approvals.entrySet()) {
args.add("--" + approval.getKey().get());
args.add(Short.toString(approval.getValue().get()));
addArg(args, "--" + approval.getKey().get(), Short.toString(approval.getValue().get()));
}
runHook(getRepo(change), args);
@@ -278,18 +270,12 @@ public class ChangeHookRunner {
final List<String> args = new ArrayList<String>();
args.add(changeMergedHook.getAbsolutePath());
args.add("--change");
args.add(event.change.id);
args.add("--change-url");
args.add(event.change.url);
args.add("--project");
args.add(event.change.project);
args.add("--branch");
args.add(event.change.branch);
args.add("--submitter");
args.add(getDisplayName(account));
args.add("--commit");
args.add(event.patchSet.revision);
addArg(args, "--change", event.change.id);
addArg(args, "--change-url", event.change.url);
addArg(args, "--project", event.change.project);
addArg(args, "--branch", event.change.branch);
addArg(args, "--submitter", getDisplayName(account));
addArg(args, "--commit", event.patchSet.revision);
runHook(getRepo(change), args);
}
@@ -312,18 +298,12 @@ public class ChangeHookRunner {
final List<String> args = new ArrayList<String>();
args.add(changeAbandonedHook.getAbsolutePath());
args.add("--change");
args.add(event.change.id);
args.add("--change-url");
args.add(event.change.url);
args.add("--project");
args.add(event.change.project);
args.add("--branch");
args.add(event.change.branch);
args.add("--abandoner");
args.add(getDisplayName(account));
args.add("--reason");
args.add(reason == null ? "" : reason);
addArg(args, "--change", event.change.id);
addArg(args, "--change-url", event.change.url);
addArg(args, "--project", event.change.project);
addArg(args, "--branch", event.change.branch);
addArg(args, "--abandoner", getDisplayName(account));
addArg(args, "--reason", reason == null ? "" : reason);
runHook(getRepo(change), args);
}