Simplify adding comments Replace nested ifs with switches and extract helper methods. This refactoring facilitates the handling of multiple servers which is done in a follow up change. Change-Id: Ie7ee92dc9a303d6869fb043dddefb216a47dad34
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionExecutor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionExecutor.java index c3e0421..fdf1e8e 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionExecutor.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionExecutor.java
@@ -50,20 +50,24 @@ this.logEventFactory = logEventFactory; } + private Action getAction(String actionName) { + switch (actionName) { + case "add-comment": + return addCommentFactory.create(); + case "add-standard-comment": + return addStandardCommentFactory.create(); + case "add-soy-comment": + return addSoyCommentFactory.create(); + case "log-event": + return logEventFactory.create(); + default: + return null; + } + } + public void execute(String issue, ActionRequest actionRequest, Set<Property> properties) { try { - String name = actionRequest.getName(); - Action action = null; - if ("add-comment".equals(name)) { - action = addCommentFactory.create(); - } else if ("add-standard-comment".equals(name)) { - action = addStandardCommentFactory.create(); - } else if ("add-soy-comment".equals(name)) { - action = addSoyCommentFactory.create(); - } else if ("log-event".equals(name)) { - action = logEventFactory.create(); - } - + Action action = getAction(actionRequest.getName()); if (action == null) { its.performAction(issue, actionRequest.getUnparsed()); } else {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddSoyComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddSoyComment.java index bbad6e9..2396234 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddSoyComment.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddSoyComment.java
@@ -115,17 +115,18 @@ @Override public void execute(String issue, ActionRequest actionRequest, Set<Property> properties) throws IOException { - SoyFileSet.Builder builder = SoyFileSet.builder(); - String template = null; - String templateName = actionRequest.getParameter(1); - if (templateName.isEmpty()) { - log.error("No template name given in {}", actionRequest); - } else { - template = templateName; - } - if (!Strings.isNullOrEmpty(template)) { - String comment = soyTextTemplate(builder, template, properties); + String comment = buildComment(actionRequest, properties); + if (!Strings.isNullOrEmpty(comment)) { its.addComment(issue, comment); } } + + private String buildComment(ActionRequest actionRequest, Set<Property> properties) { + String template = actionRequest.getParameter(1); + if (!template.isEmpty()) { + return soyTextTemplate(SoyFileSet.builder(), template, properties); + } + log.error("No template name given in {}", actionRequest); + return ""; + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddStandardComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddStandardComment.java index 8a7a09f..4777d4b 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddStandardComment.java +++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/action/AddStandardComment.java
@@ -77,26 +77,16 @@ return ""; } - private String getCommentChangeAbandoned(Map<String, String> map) { - return getCommentChangeEvent("abandoned", "abandoner", map); - } - - private String getCommentChangeMerged(Map<String, String> map) { - return getCommentChangeEvent("merged", "submitter", map); - } - - private String getCommentChangeRestored(Map<String, String> map) { - return getCommentChangeEvent("restored", "restorer", map); - } - - private String getCommentPatchSetCreated(Map<String, String> map) { - return getCommentChangeEvent("had a related patch set uploaded", "uploader", map); - } - @Override public void execute(String issue, ActionRequest actionRequest, Set<Property> properties) throws IOException { - String comment = ""; + String comment = buildComment(properties); + if (!Strings.isNullOrEmpty(comment)) { + its.addComment(issue, comment); + } + } + + private String buildComment(Set<Property> properties) { Map<String, String> map = Maps.newHashMap(); for (Property property : properties) { String current = property.getValue(); @@ -109,18 +99,17 @@ map.put(key, old + current); } } - String eventType = map.get("event-type"); - if ("change-abandoned".equals(eventType)) { - comment = getCommentChangeAbandoned(map); - } else if ("change-merged".equals(eventType)) { - comment = getCommentChangeMerged(map); - } else if ("change-restored".equals(eventType)) { - comment = getCommentChangeRestored(map); - } else if ("patchset-created".equals(eventType)) { - comment = getCommentPatchSetCreated(map); - } - if (!Strings.isNullOrEmpty(comment)) { - its.addComment(issue, comment); + switch (map.get("event-type")) { + case "change-abandoned": + return getCommentChangeEvent("abandoned", "abandoner", map); + case "change-merged": + return getCommentChangeEvent("merged", "submitter", map); + case "change-restored": + return getCommentChangeEvent("restored", "restorer", map); + case "patchset-created": + return getCommentChangeEvent("had a related patch set uploaded", "uploader", map); + default: + return ""; } } }