Preparation for allowing chaining actions into one XMLRPC request
Bugzilla does not allow to set a bug to a closed state without also
providing a resolution in the same request. Our current approach
however only allows to update one field per XMLRPC request. Therefore,
we decouple user visible actions from the fields provided by bugzilla
and can introduce aggregated actions in a follow-up commit.
Change-Id: I5de9139c8deff908b75a504ded416f38f3f1153f
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaClient.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaClient.java
index c05cad1..6b6609a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaClient.java
@@ -21,6 +21,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.googlesource.gerrit.plugins.hooks.its.InvalidTransitionException;
import com.j2bugzilla.base.Bug;
import com.j2bugzilla.base.BugzillaConnector;
import com.j2bugzilla.base.BugzillaException;
@@ -77,25 +78,41 @@
connector.executeMethod(bugComment);
}
- public void performAction(final String bugId, final String actionName,
- final String actionValue) throws BugzillaException {
- Bug bug = getBug(bugId);
- if (validateAction(actionName, actionValue)) {
- if (actionName.equals("status")) {
- bug.setStatus(actionValue);
- } else if (actionName.equals("resolution")) {
- bug.setResolution(actionValue);
- }
+ private void performSimpleActionChainable(final Bug bug, final String actionName,
+ final String actionValue) throws BugzillaException,
+ InvalidTransitionException {
+ if ("status".equals(actionName)) {
+ assertLegalValue(Fields.STATUS, actionValue);
+ bug.setStatus(actionValue);
+ } else if ("resolution".equals(actionName)) {
+ assertLegalValue(Fields.RESOLUTION, actionValue);
+ bug.setResolution(actionValue);
} else {
- log.warn( "Action '" + actionName + "' with value '" + actionValue
- + "' is not valid. Skipping action on issue " + bugId + "." );
+ throw new InvalidTransitionException("Simple action " + actionName
+ + " is not known");
+ }
+ }
+
+ public void performAction(final String bugId, final String actionName,
+ final String actionValue) throws BugzillaException,
+ InvalidTransitionException {
+ Bug bug = getBug(bugId);
+ if ("status".equals(actionName) || "resolution".equals(actionName)) {
+ performSimpleActionChainable(bug, actionName, actionValue);
+ } else {
+ throw new InvalidTransitionException("Action " + actionName + " is not"
+ + " known");
}
connector.executeMethod(new UpdateBug(bug));
}
- private boolean validateAction(String actionName, String actionValue) throws BugzillaException {
- Fields field = getFields().get(actionName);
- return field != null && getLegalValues(field).contains(actionValue);
+ private void assertLegalValue(Fields field, String actionValue)
+ throws BugzillaException, InvalidTransitionException {
+ if (!getLegalValues(field).contains(actionValue)) {
+ throw new InvalidTransitionException( "The value '" + actionValue
+ + "' is not an allowed value for bugzilla's " + field.getInternalName()
+ + " field");
+ }
}
public String getServerVersion() throws BugzillaException {
@@ -108,13 +125,6 @@
return xmlRpcUrl;
}
- public Map<String, Fields> getFields() {
- Map<String, Fields> fields = new HashMap<String, Fields>();
- fields.put("status", Fields.STATUS);
- fields.put("resolution", Fields.RESOLUTION);
- return fields;
- }
-
private Set<String> getLegalValues(Fields field) throws BugzillaException {
if (legalFieldValues.get(field) == null) {
GetLegalValues getValues = new GetLegalValues(field);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaItsFacade.java b/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaItsFacade.java
index 7088ab0..264fad6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaItsFacade.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/hooks/bz/BugzillaItsFacade.java
@@ -30,7 +30,6 @@
import com.j2bugzilla.base.Bug;
import com.j2bugzilla.base.BugzillaException;
import com.j2bugzilla.base.ConnectionException;
-import com.j2bugzilla.rpc.GetLegalValues.Fields;
public class BugzillaItsFacade implements ItsFacade {
public static final String ITS_NAME_BUGZILLA = "bugzilla";
@@ -110,31 +109,7 @@
private void doPerformAction(final String bugId, final String fieldName, final String fieldValue)
throws BugzillaException, IOException {
- String actionKey = null;
- Map<String, Fields> fields = client().getFields();
- for (Map.Entry<String, Fields> field : fields.entrySet()) {
- if (field.getKey().equalsIgnoreCase(fieldName)) {
- actionKey = field.getKey();
- }
- }
-
- if (actionKey != null) {
- log.debug("Executing action " + actionKey + " on issue " + bugId);
- client().performAction(bugId, actionKey, fieldValue);
- } else {
- StringBuilder sb = new StringBuilder();
- for (Map.Entry<String, Fields> action : fields.entrySet()) {
- if (sb.length() > 0) sb.append(',');
- sb.append('\'');
- sb.append(action.getKey());
- sb.append('\'');
- }
-
- log.error("Action " + fieldName
- + " not found within available actions: " + sb);
- throw new InvalidTransitionException("Action " + fieldName
- + " not executable on issue " + bugId);
- }
+ client().performAction(bugId, fieldName.toLowerCase(), fieldValue);
}
@Override
