Merge branch 'stable-3.1'
* stable-3.1:
Revert "Log matched issues at fine level to aid debugging"
Extend CommitMessageFetcher to handle non-commit objects
Fix typo in documentation RevUpdatedEvents -> RefUpdatedEvents
Check if approvals array is null before iterating
Log matched issues at fine level to aid debugging
Switch from deprecated SoyTofu to current SoySauce
Avoid NPE in AddSoyComment if no template file given
Add test for AddSoyComment
Allow to log rendered Soy templates
Switch to Flogger
Document default implementations in ItsFacade
Fix parameter specification for Soy template example
Drop 'autoescape' from Soy template example
Mute warning when actions config file is missing
Change-Id: Ibaa429fb95f0b0ebeec795cb92f59a8d79fd6b99
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
index 20c6c4a..af5e9a9 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsConfig.java
@@ -16,6 +16,7 @@
import static java.util.stream.Collectors.toList;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
@@ -44,13 +45,11 @@
import java.util.Optional;
import java.util.regex.Pattern;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class ItsConfig {
private static final String PLUGIN = "plugin";
- private static final Logger log = LoggerFactory.getLogger(ItsConfig.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final String pluginName;
private final ProjectCache projectCache;
@@ -89,17 +88,16 @@
|| event instanceof RefUpdatedEvent) {
return isEnabled(event.getProjectNameKey(), event.getRefName());
}
- log.debug("Event {} not recognised and ignored", event);
+ logger.atFine().log("Event %s not recognised and ignored", event);
return false;
}
public boolean isEnabled(Project.NameKey projectNK, String refName) {
Optional<ProjectState> projectState = projectCache.get(projectNK);
if (!projectState.isPresent()) {
- log.error(
- "Failed to check if {} is enabled for project {}: Project not found",
- pluginName,
- projectNK.get());
+ logger.atSevere().log(
+ "Failed to check if %s is enabled for project %s: Project not found",
+ pluginName, projectNK.get());
return false;
}
return isEnforcedByAnyParentProject(refName, projectState.get())
@@ -260,7 +258,8 @@
try {
return pluginCfgFactory.getFromProjectConfigWithInheritance(projectName, pluginName);
} catch (NoSuchProjectException e) {
- log.error("Cannot access " + projectName + " configuration for plugin " + pluginName, e);
+ logger.atSevere().withCause(e).log(
+ "Cannot access %s configuration for plugin %s", projectName, pluginName);
}
}
return new PluginConfig(pluginName, new Config());
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsFacade.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsFacade.java
index db2857b..6531ed7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsFacade.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/ItsFacade.java
@@ -31,6 +31,14 @@
public void addComment(String issueId, String comment) throws IOException;
+ /**
+ * Sets a field on the issue to an event property.
+ *
+ * @param issueId The issue to set the field on
+ * @param value The name of the property to set the issues's field to
+ * @param fieldId The field on the issue to set
+ * @throws IOException if an error occurred
+ */
default void addValueToField(String issueId, String value, String fieldId) throws IOException {
throw new UnsupportedOperationException(
"add-value-to-field is not currently implemented by " + getClass());
@@ -38,6 +46,13 @@
public void performAction(String issueId, String actionName) throws IOException;
+ /**
+ * Creates a new version on a given ITS.
+ *
+ * @param itsProject the ITS to create the version on
+ * @param version the version to create
+ * @throws IOException if an error occurred
+ */
default void createVersion(String itsProject, String version) throws IOException {
throw new UnsupportedOperationException("create-version is not implemented by " + getClass());
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/NoopItsFacade.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/NoopItsFacade.java
index ecf3a6f..ba7fb14 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/its/NoopItsFacade.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/its/NoopItsFacade.java
@@ -14,73 +14,55 @@
package com.googlesource.gerrit.plugins.its.base.its;
+import com.google.common.flogger.FluentLogger;
import java.io.IOException;
import java.net.URL;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** An ITS facade doing nothing, it's configured when no ITS are referenced in config */
public class NoopItsFacade implements ItsFacade {
-
- private Logger log = LoggerFactory.getLogger(NoopItsFacade.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Override
public void addComment(String issueId, String comment) throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("addComment({},{})", issueId, comment);
- }
+ logger.atFine().log("addComment(%s,%s)", issueId, comment);
}
@Override
public void addValueToField(String issueId, String value, String fieldId) throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("addValueToField({},{},{})", issueId, fieldId, value);
- }
+ logger.atFine().log("addValueToField(%s,%s,%s)", issueId, fieldId, value);
}
@Override
public void addRelatedLink(String issueId, URL relatedUrl, String description)
throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("addRelatedLink({},{},{})", issueId, relatedUrl, description);
- }
+ logger.atFine().log("addRelatedLink(%s,%s,%s)", issueId, relatedUrl, description);
}
@Override
public boolean exists(String issueId) throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("exists({})", issueId);
- }
+ logger.atFine().log("exists(%s)", issueId);
return false;
}
@Override
public void performAction(String issueId, String actionName) throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("performAction({},{})", issueId, actionName);
- }
+ logger.atFine().log("performAction(%s,%s)", issueId, actionName);
}
@Override
public void createVersion(String itsProject, String version) {
- if (log.isDebugEnabled()) {
- log.debug("createVersion({},{})", itsProject, version);
- }
+ logger.atFine().log("createVersion(%s,%s)", itsProject, version);
}
@Override
public String healthCheck(Check check) throws IOException {
- if (log.isDebugEnabled()) {
- log.debug("healthCheck()");
- }
+ logger.atFine().log("healthCheck()");
return "{\"status\"=\"ok\",\"system\"=\"not configured\",}";
}
@Override
public String createLinkForWebui(String url, String text) {
- if (log.isDebugEnabled()) {
- log.debug("createLinkForWebui({},{})", url, text);
- }
+ logger.atFine().log("createLinkForWebui(%s,%s)", url, text);
return "";
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
index b34bd55..1d5188d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
@@ -1,5 +1,6 @@
package com.googlesource.gerrit.plugins.its.base.util;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
@@ -7,12 +8,11 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevWalk;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class CommitMessageFetcher {
- private static final Logger log = LoggerFactory.getLogger(CommitMessageFetcher.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final GitRepositoryManager repoManager;
@@ -21,23 +21,33 @@
this.repoManager = repoManager;
}
- public String fetch(String projectName, String commitId) throws IOException {
+ public String fetch(String projectName, String objectId) throws IOException {
try (Repository repo = repoManager.openRepository(Project.nameKey(projectName))) {
try (RevWalk revWalk = new RevWalk(repo)) {
- RevCommit commit = revWalk.parseCommit(ObjectId.fromString(commitId));
- return commit.getFullMessage();
+ RevObject obj = revWalk.peel(revWalk.parseAny(ObjectId.fromString(objectId)));
+ if (obj instanceof RevCommit) {
+ return ((RevCommit) obj).getFullMessage();
+ }
+ // objectId was found, but it's not a commit.
+ // Since the objectId was found, it's nothing to worry about and we do not need to alert the
+ // user. We silently return the empty string as blobs, trees, ... do not have a proper
+ // commit message.
+ //
+ // Parsing a non-commit objectId (and reaching this point) will happen for example on NoteDB
+ // sites when Gerrit updates `refs/sequences/changes` (which does not point at a commit, but
+ // a blob) on All-Projects and the corresponding RefUpdatedEvent gets processed.
+ return "";
}
}
}
- public String fetchGuarded(String projectName, String commitId) {
+ public String fetchGuarded(String projectName, String objectId) {
String ret = "";
try {
- ret = fetch(projectName, commitId);
+ ret = fetch(projectName, objectId);
} catch (IOException e) {
- log.error(
- "Could not fetch commit message for commit " + commitId + " of project " + projectName,
- e);
+ logger.atSevere().withCause(e).log(
+ "Could not fetch commit message for commit %s of project %s", objectId, projectName);
}
return ret;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
index 44611fe..6131ed4 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/IssueExtractor.java
@@ -5,6 +5,7 @@
import com.google.common.base.Strings;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -19,11 +20,9 @@
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class IssueExtractor {
- private static final Logger log = LoggerFactory.getLogger(IssueExtractor.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final CommitMessageFetcher commitMessageFetcher;
private final PatchSetDb db;
@@ -80,7 +79,7 @@
Pattern pattern = itsConfig.getIssuePattern();
if (pattern == null) return new String[] {};
- log.debug("Matching '{}' against {}", haystack, pattern.pattern());
+ logger.atFine().log("Matching '%s' against '%s'", haystack, pattern.pattern());
Set<String> issues = Sets.newHashSet();
Matcher matcher = pattern.matcher(haystack);
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractor.java
index bf9dc0c..4b8a7bd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractor.java
@@ -130,8 +130,9 @@
CommentAddedEvent event, Map<String, String> common) {
common.putAll(propertyAttributeExtractor.extractFrom(event.author.get(), "commenter"));
common.put("comment", event.comment);
- if (event.approvals != null) {
- for (ApprovalAttribute approvalAttribute : event.approvals.get()) {
+ ApprovalAttribute[] approvals = event.approvals.get();
+ if (approvals != null) {
+ for (ApprovalAttribute approvalAttribute : approvals) {
common.putAll(propertyAttributeExtractor.extractFrom(approvalAttribute));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
index c5799d2..074b882 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/validation/ItsValidateComment.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.its.base.validation;
import com.google.common.collect.Lists;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.events.CommitReceivedEvent;
@@ -30,12 +31,10 @@
import java.util.Collections;
import java.util.List;
import org.eclipse.jgit.revwalk.RevCommit;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class ItsValidateComment implements CommitValidationListener {
- private static final Logger log = LoggerFactory.getLogger(ItsValidateComment.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject private ItsFacade client;
@@ -80,7 +79,7 @@
"Failed to check whether or not issue "
+ issueId
+ " exists, due to connectivity issue. Commit will be accepted.";
- log.warn(synopsis, e);
+ logger.atWarning().withCause(e).log(synopsis);
details = e.toString();
existenceCheckResult = ItsExistenceCheckResult.CONNECTIVITY_FAILURE;
ret.add(commitValidationFailure(synopsis, details, existenceCheckResult));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionController.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionController.java
index 4af8f1b..f9214c6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionController.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ActionController.java
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.server.events.Event;
import com.google.gerrit.server.events.EventListener;
import com.google.gerrit.server.events.RefEvent;
@@ -23,8 +24,6 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Controller that takes actions according to {@code ChangeEvents@}.
@@ -34,7 +33,7 @@
*/
public class ActionController implements EventListener {
- private static final Logger log = LoggerFactory.getLogger(ActionController.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final PropertyExtractor propertyExtractor;
private final RuleBase ruleBase;
@@ -90,8 +89,8 @@
}
if (!projectProperties.containsKey("its-project")) {
String project = projectProperties.get("project");
- log.error(
- "Could not process project event. No its-project associated with project {}. "
+ logger.atSevere().log(
+ "Could not process project event. No its-project associated with project %s. "
+ "Did you forget to configure the ITS project association in project.config?",
project);
return;
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 44f5d73..789b7e8 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
@@ -14,6 +14,7 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.PluginName;
@@ -22,12 +23,10 @@
import com.googlesource.gerrit.plugins.its.base.its.ItsFacadeFactory;
import java.io.IOException;
import java.util.Map;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** Executes an {@link ActionRequest} */
public class ActionExecutor {
- private static final Logger log = LoggerFactory.getLogger(ActionExecutor.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final ItsFacadeFactory itsFactory;
private final AddComment.Factory addCommentFactory;
@@ -100,7 +99,7 @@
execute(action, issue, actionRequest, properties);
}
} catch (IOException e) {
- log.error("Error while executing action " + actionRequest, e);
+ logger.atSevere().withCause(e).log("Error while executing action %s", actionRequest);
}
}
@@ -116,7 +115,7 @@
String actionName = actionRequest.getName();
Action action = getAction(actionName);
if (action == null) {
- log.debug("No action found for name {}", actionName);
+ logger.atFine().log("No action found for name %s", actionName);
return;
}
if (action.getType() != ActionType.PROJECT) {
@@ -124,7 +123,7 @@
}
execute(action, itsProject, actionRequest, properties);
} catch (IOException e) {
- log.error("Error while executing action " + actionRequest, e);
+ logger.atSevere().withCause(e).log("Error while executing action %s", actionRequest);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddPropertyToFieldParametersExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddPropertyToFieldParametersExtractor.java
index 7a259f1..3b1f356 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddPropertyToFieldParametersExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddPropertyToFieldParametersExtractor.java
@@ -15,17 +15,14 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import java.util.Arrays;
import java.util.Map;
import java.util.Optional;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class AddPropertyToFieldParametersExtractor {
-
- private static final Logger log =
- LoggerFactory.getLogger(AddPropertyToFieldParametersExtractor.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject
public AddPropertyToFieldParametersExtractor() {}
@@ -38,26 +35,26 @@
ActionRequest actionRequest, Map<String, String> properties) {
String[] parameters = actionRequest.getParameters();
if (parameters.length != 2) {
- log.error(
- "Wrong number of received parameters. Received parameters are {}. Exactly two parameters are expected. The first one is the ITS field id, the second one is the event property id",
+ logger.atSevere().log(
+ "Wrong number of received parameters. Received parameters are %s. Exactly two parameters are expected. The first one is the ITS field id, the second one is the event property id",
Arrays.toString(parameters));
return Optional.empty();
}
String propertyId = parameters[0];
if (Strings.isNullOrEmpty(propertyId)) {
- log.error("Received property id is blank");
+ logger.atSevere().log("Received property id is blank");
return Optional.empty();
}
String fieldId = parameters[1];
if (Strings.isNullOrEmpty(fieldId)) {
- log.error("Received field id is blank");
+ logger.atSevere().log("Received field id is blank");
return Optional.empty();
}
if (!properties.containsKey(propertyId)) {
- log.error("No event property found for id {}", propertyId);
+ logger.atSevere().log("No event property found for id %s", propertyId);
return Optional.empty();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyComment.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyComment.java
index c29d622..d3f73a2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyComment.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyComment.java
@@ -15,13 +15,12 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import com.google.common.io.CharStreams;
import com.google.inject.Inject;
import com.google.inject.ProvisionException;
import com.google.template.soy.SoyFileSet;
-import com.google.template.soy.SoyFileSet.Builder;
-import com.google.template.soy.data.SanitizedContent;
-import com.google.template.soy.tofu.SoyTofu;
+import com.google.template.soy.jbcsrc.api.SoySauce.Renderer;
import com.googlesource.gerrit.plugins.its.base.ItsPath;
import com.googlesource.gerrit.plugins.its.base.its.ItsFacade;
import java.io.IOException;
@@ -31,8 +30,6 @@
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Adds a short predefined comments to an issue.
@@ -40,7 +37,7 @@
* <p>Comments are added for merging, abandoning, restoring of changes and adding of patch sets.
*/
public class AddSoyComment extends IssueAction {
- private static final Logger log = LoggerFactory.getLogger(AddSoyComment.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
AddSoyComment create();
@@ -54,11 +51,9 @@
this.templateDir = itsPath.resolve("templates");
}
- private String soyTemplate(
- SoyFileSet.Builder builder,
- String template,
- SanitizedContent.ContentKind kind,
- Map<String, String> properties) {
+ private String soyTextTemplate(
+ SoyFileSet.Builder builder, String template, Map<String, String> properties) {
+
Path templatePath = templateDir.resolve(template + ".soy");
String content;
@@ -70,18 +65,15 @@
}
builder.add(content, templatePath.toAbsolutePath().toString());
- SoyTofu.Renderer renderer =
+ Renderer renderer =
builder
.build()
- .compileToTofu()
- .newRenderer("etc.its.templates." + template)
- .setContentKind(kind)
+ .compileTemplates()
+ .renderTemplate("etc.its.templates." + template)
.setData(properties);
- return renderer.render();
- }
-
- private String soyTextTemplate(Builder builder, String template, Map<String, String> properties) {
- return soyTemplate(builder, template, SanitizedContent.ContentKind.TEXT, properties);
+ String rendered = renderer.renderText().get();
+ logger.atFinest().log("Rendered template %s to:\n%s", templatePath, rendered);
+ return rendered;
}
@Override
@@ -96,10 +88,10 @@
private String buildComment(ActionRequest actionRequest, Map<String, String> properties) {
String template = actionRequest.getParameter(1);
- if (!template.isEmpty()) {
+ if (!Strings.isNullOrEmpty(template)) {
return soyTextTemplate(SoyFileSet.builder(), template, properties);
}
- log.error("No template name given in {}", actionRequest);
+ logger.atSevere().log("No template name given in %s", actionRequest);
return "";
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/CreateVersionFromPropertyParametersExtractor.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/CreateVersionFromPropertyParametersExtractor.java
index 1eb01ce..3f81485 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/CreateVersionFromPropertyParametersExtractor.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/CreateVersionFromPropertyParametersExtractor.java
@@ -15,17 +15,14 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
import com.google.common.base.Strings;
+import com.google.common.flogger.FluentLogger;
import java.util.Arrays;
import java.util.Map;
import java.util.Optional;
import javax.inject.Inject;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
class CreateVersionFromPropertyParametersExtractor {
-
- private static final Logger log =
- LoggerFactory.getLogger(CreateVersionFromPropertyParametersExtractor.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
@Inject
public CreateVersionFromPropertyParametersExtractor() {}
@@ -34,20 +31,20 @@
ActionRequest actionRequest, Map<String, String> properties) {
String[] parameters = actionRequest.getParameters();
if (parameters.length != 1) {
- log.error(
- "Wrong number of received parameters. Received parameters are {}. Only one parameter is expected, the property id.",
+ logger.atSevere().log(
+ "Wrong number of received parameters. Received parameters are %s. Only one parameter is expected, the property id.",
Arrays.toString(parameters));
return Optional.empty();
}
String propertyId = parameters[0];
if (Strings.isNullOrEmpty(propertyId)) {
- log.error("Received property id is blank");
+ logger.atSevere().log("Received property id is blank");
return Optional.empty();
}
if (!properties.containsKey(propertyId)) {
- log.error("No event property found for id {}", propertyId);
+ logger.atSevere().log("No event property found for id %s", propertyId);
return Optional.empty();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheImpl.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheImpl.java
index 4dc46d3..e55a6dd 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheImpl.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheImpl.java
@@ -17,6 +17,7 @@
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -34,12 +35,10 @@
import java.util.List;
import java.util.concurrent.ExecutionException;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class ItsRulesProjectCacheImpl implements ItsRulesProjectCache {
- private static final Logger log = LoggerFactory.getLogger(ItsRulesProjectCacheImpl.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String CACHE_NAME = "its_rules_project";
private final LoadingCache<String, List<Rule>> cache;
@@ -54,7 +53,8 @@
try {
return cache.get(projectName);
} catch (ExecutionException e) {
- log.warn("Cannot get project specific rules for project {}", projectName, e);
+ logger.atWarning().withCause(e).log(
+ "Cannot get project specific rules for project %s", projectName);
return ImmutableList.of();
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheRefresher.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheRefresher.java
index 6d19849..75b91f7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheRefresher.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/ItsRulesProjectCacheRefresher.java
@@ -1,16 +1,15 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.Inject;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
public class ItsRulesProjectCacheRefresher implements GitReferenceUpdatedListener {
- private static final Logger log = LoggerFactory.getLogger(ItsRulesProjectCacheRefresher.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final GerritApi gApi;
private final ItsRulesProjectCache itsRuleProjectCache;
@@ -33,7 +32,7 @@
itsRuleProjectCache.evict(childProject.name);
}
} catch (RestApiException e) {
- log.warn("Unable to evict ITS rules cache", e);
+ logger.atWarning().withCause(e).log("Unable to evict ITS rules cache");
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/LogEvent.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/LogEvent.java
index 6eec9a4..f345227 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/LogEvent.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/LogEvent.java
@@ -14,13 +14,12 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
+import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.its.base.its.ItsFacade;
import java.io.IOException;
import java.util.Map;
import java.util.Map.Entry;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/**
* Dumps the event's properties to the log.
@@ -28,7 +27,7 @@
* <p>This event helps when developing rules as available properties become visible.
*/
public class LogEvent extends IssueAction {
- private static final Logger log = LoggerFactory.getLogger(LogEvent.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private enum Level {
ERROR,
@@ -56,23 +55,25 @@
public LogEvent() {}
private void logProperty(Level level, Entry<String, String> property) {
- String message = String.format("[%s = %s]", property.getKey(), property.getValue());
+ final java.util.logging.Level logLevel;
switch (level) {
case ERROR:
- log.error(message);
+ logLevel = java.util.logging.Level.SEVERE;
break;
case WARN:
- log.warn(message);
+ logLevel = java.util.logging.Level.WARNING;
break;
case INFO:
- log.info(message);
+ logLevel = java.util.logging.Level.INFO;
break;
case DEBUG:
- log.debug(message);
+ logLevel = java.util.logging.Level.FINE;
break;
default:
- log.error("Undefined log level.");
+ logger.atSevere().log("Undefined log level.");
+ return;
}
+ logger.at(logLevel).log("[%s = %s]", property.getKey(), property.getValue());
}
@Override
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
index b6e4ad6..b051dfe 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/workflow/RuleBase.java
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.plugins.its.base.workflow;
import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.its.base.GlobalRulesFileName;
import com.googlesource.gerrit.plugins.its.base.ItsPath;
@@ -29,12 +30,10 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
/** Collection and matcher against {@link Rule}s. */
public class RuleBase {
- private static final Logger log = LoggerFactory.getLogger(RuleBase.class);
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final File globalRuleFile;
private final File itsSpecificRuleFile;
@@ -81,7 +80,7 @@
cfg.load();
return rulesConfigReader.getRulesFromConfig(cfg);
} catch (IOException | ConfigInvalidException e) {
- log.error("Invalid ITS action configuration", e);
+ logger.atSevere().withCause(e).log("Invalid ITS action configuration");
}
}
return Collections.emptyList();
@@ -98,12 +97,10 @@
Collection<Rule> fromProjectConfig = rulesProjectCache.get(projectName);
Collection<Rule> rulesToAdd = !fromProjectConfig.isEmpty() ? fromProjectConfig : rules;
if (rulesToAdd.isEmpty() && !globalRuleFile.exists() && !itsSpecificRuleFile.exists()) {
- log.warn(
- "Neither global rule file {} nor Its specific rule file {} exist and no rules are "
- + "configured for project {}. Please configure rules.",
- globalRuleFile,
- itsSpecificRuleFile,
- projectName);
+ logger.atFine().log(
+ "Neither global rule file %s nor Its specific rule file %s exist and no rules are "
+ + "configured for project %s. Please configure rules.",
+ globalRuleFile, itsSpecificRuleFile, projectName);
return Collections.emptyList();
}
Collection<ActionRequest> actions = new ArrayList<>();
diff --git a/src/main/resources/Documentation/config-rulebase-common.md b/src/main/resources/Documentation/config-rulebase-common.md
index 50e1023..95eecde 100644
--- a/src/main/resources/Documentation/config-rulebase-common.md
+++ b/src/main/resources/Documentation/config-rulebase-common.md
@@ -281,7 +281,7 @@
`added@<Association-Value>`
: (only for events that allow to determine the patch set number.
So for example, this `association` property is not set for
- RevUpdatedEvents)
+ RefUpdatedEvents)
issue id occurs at `<Association-Value>` in the most recent
patch set of the change, and either the event is for patch set
@@ -658,12 +658,9 @@
```
{namespace etc.its.templates}
-
-/**
- * @param changeNumber
- * @param formatChangeUrl
- */
-{template .TemplateName autoescape="strict" kind="text"}
+{template .TemplateName kind="text"}
+ {@param changeNumber: string}
+ {@param formatChangeUrl: string}
inline Comment for change {$changeNumber} added. See {$formatChangeUrl}
{/template}
```
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
index 03dda08..e7a137a 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/LoggingMockingTestCase.java
@@ -22,9 +22,9 @@
import com.googlesource.gerrit.plugins.its.base.testutil.log.LogUtil;
import java.sql.Timestamp;
import java.util.Iterator;
+import java.util.logging.LogRecord;
import junit.framework.TestCase;
import org.apache.log4j.Level;
-import org.apache.log4j.spi.LoggingEvent;
import org.junit.After;
public abstract class LoggingMockingTestCase extends TestCase {
@@ -34,22 +34,21 @@
protected final Change.Id testChangeId = Change.id(1);
protected final Account.Id testAccountId = Account.id(1);
- private java.util.Collection<LoggingEvent> loggedEvents;
+ private java.util.Collection<LogRecord> records;
protected final void assertLogMessageContains(String needle, Level level) {
- LoggingEvent hit = null;
- Iterator<LoggingEvent> iter = loggedEvents.iterator();
+ LogRecord hit = null;
+ Iterator<LogRecord> iter = records.iterator();
while (hit == null && iter.hasNext()) {
- LoggingEvent event = iter.next();
- if (event.getRenderedMessage().contains(needle)) {
- if (level == null || level.equals(event.getLevel())) {
- hit = event;
+ LogRecord record = iter.next();
+ if (record.getMessage().contains(needle)) {
+ if (level == null || LogUtil.equalLevels(record.getLevel(), level)) {
+ hit = record;
}
}
}
assertNotNull("Could not find log message containing '" + needle + "'", hit);
- assertTrue(
- "Could not remove log message containing '" + needle + "'", loggedEvents.remove(hit));
+ assertTrue("Could not remove log message containing '" + needle + "'", records.remove(hit));
}
protected final void assertLogMessageContains(String needle) {
@@ -57,31 +56,33 @@
}
protected final void assertLogThrowableMessageContains(String needle) {
- LoggingEvent hit = null;
- Iterator<LoggingEvent> iter = loggedEvents.iterator();
+ LogRecord hit = null;
+ Iterator<LogRecord> iter = records.iterator();
while (hit == null && iter.hasNext()) {
- LoggingEvent event = iter.next();
+ LogRecord record = iter.next();
- if (event.getThrowableInformation().getThrowable().toString().contains(needle)) {
- hit = event;
+ Throwable t = record.getThrown();
+ if (t != null && t.toString().contains(needle)) {
+ hit = record;
}
}
assertNotNull("Could not find log message with a Throwable containing '" + needle + "'", hit);
assertTrue(
"Could not remove log message with a Throwable containing '" + needle + "'",
- loggedEvents.remove(hit));
+ records.remove(hit));
}
// As the PowerMock runner does not pass through runTest, we inject log
// verification through @After
@After
public final void assertNoUnassertedLogEvents() {
- if (loggedEvents.size() > 0) {
- LoggingEvent event = loggedEvents.iterator().next();
+ if (records.size() > 0) {
+ LogRecord record = records.iterator().next();
String msg = "Found untreated logged events. First one is:\n";
- msg += event.getRenderedMessage();
- if (event.getThrowableInformation() != null) {
- msg += "\n" + event.getThrowableInformation().getThrowable();
+ msg += record.getMessage();
+ Throwable t = record.getThrown();
+ if (t != null) {
+ msg += "\n" + t;
}
fail(msg);
}
@@ -90,7 +91,7 @@
@Override
public void setUp() throws Exception {
super.setUp();
- loggedEvents = Lists.newArrayList();
+ records = Lists.newArrayList();
// The logger we're interested is class name without the trailing "Test".
// While this is not the most general approach it is sufficient for now,
@@ -98,7 +99,7 @@
// to check.
String logName = this.getClass().getCanonicalName();
logName = logName.substring(0, logName.length() - 4);
- LogUtil.logToCollection(logName, loggedEvents);
+ LogUtil.logToCollection(logName, records, Level.DEBUG);
}
@Override
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/CollectionAppender.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/CollectionAppender.java
index c122676..8d918b2 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/CollectionAppender.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/CollectionAppender.java
@@ -17,18 +17,19 @@
import com.google.common.collect.Lists;
import java.util.Collection;
import java.util.LinkedList;
+import java.util.logging.LogRecord;
import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.spi.LoggingEvent;
/** Log4j appender that logs into a list */
public class CollectionAppender extends AppenderSkeleton {
- private Collection<LoggingEvent> events;
+ private Collection<LogRecord> events;
public CollectionAppender() {
events = new LinkedList<>();
}
- public CollectionAppender(Collection<LoggingEvent> events) {
+ public CollectionAppender(Collection<LogRecord> events) {
this.events = events;
}
@@ -39,15 +40,20 @@
@Override
protected void append(LoggingEvent event) {
- if (!events.add(event)) {
- throw new RuntimeException("Could not append event " + event);
+ LogRecord logRecord = LogUtil.logRecordFromLog4jLoggingEvent(event);
+ append(logRecord);
+ }
+
+ public void append(LogRecord record) {
+ if (!events.add(record)) {
+ throw new RuntimeException("Could not append event " + record);
}
}
@Override
public void close() {}
- public Collection<LoggingEvent> getLoggedEvents() {
+ public Collection<LogRecord> getLoggedEvents() {
return Lists.newLinkedList(events);
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
index 6cd633d..adccee8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/testutil/log/LogUtil.java
@@ -14,18 +14,123 @@
package com.googlesource.gerrit.plugins.its.base.testutil.log;
+import com.google.common.collect.ImmutableMap;
import java.util.Collection;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogRecord;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.apache.log4j.spi.LoggingEvent;
+import org.apache.log4j.spi.ThrowableInformation;
+/** Utility functions for dealing with various Loggers */
public class LogUtil {
+ private static final ImmutableMap<org.apache.log4j.Level, Level> log4jToJul =
+ new ImmutableMap.Builder<org.apache.log4j.Level, Level>()
+ .put(org.apache.log4j.Level.OFF, Level.OFF)
+ .put(org.apache.log4j.Level.FATAL, Level.SEVERE)
+ .put(org.apache.log4j.Level.ERROR, Level.SEVERE)
+ .put(org.apache.log4j.Level.WARN, Level.WARNING)
+ .put(org.apache.log4j.Level.INFO, Level.INFO)
+ .put(org.apache.log4j.Level.DEBUG, Level.FINE)
+ .put(org.apache.log4j.Level.TRACE, Level.FINEST)
+ .put(org.apache.log4j.Level.ALL, Level.FINEST)
+ .build();
+
+ /**
+ * Makes sure that Loggers for a given name add their events to a collection
+ *
+ * <p>This is useful for capturing logged events during testing and being able to run assertions
+ * on them.
+ *
+ * <p>We will never be able to cover all possible backends of the Logging flavour of the day. For
+ * now we cover Log4j and JUL. If you use a different default logging backend, please send in
+ * patches.
+ *
+ * @param logName The name of the loggers to make log to the given collection.
+ * @param collection The collection to add log events to.
+ * @param level The level the logger should be set to.
+ */
public static CollectionAppender logToCollection(
- String logName, Collection<LoggingEvent> collection) {
+ String logName, Collection<LogRecord> collection, org.apache.log4j.Level level) {
+
+ CollectionAppender appender = new CollectionAppender(collection);
+
+ logToCollectionLog4j(logName, appender, level);
+ logToCollectionJul(logName, appender, log4jToJul.get(level));
+
+ return appender;
+ }
+
+ /**
+ * Make a Log4j logger log to a given appender at a certain level
+ *
+ * @param logName The logger that should log to the appender.
+ * @param appender The appender to log to.
+ * @param level The level to user for the logger.
+ */
+ private static void logToCollectionLog4j(
+ String logName, CollectionAppender appender, org.apache.log4j.Level level) {
Logger log = LogManager.getLogger(logName);
- CollectionAppender listAppender = new CollectionAppender(collection);
+ log.setLevel(level);
log.removeAllAppenders();
- log.addAppender(listAppender);
- return listAppender;
+ log.addAppender(appender);
+ }
+
+ /**
+ * Make a java.util.logging logger log to a given appender at a certain level
+ *
+ * @param logName The logger that should log to the appender.
+ * @param appender The appender to log to.
+ * @param level The level to user for the logger.
+ */
+ private static void logToCollectionJul(String logName, CollectionAppender appender, Level level) {
+ java.util.logging.Logger julLogger = java.util.logging.Logger.getLogger(logName);
+ julLogger.setLevel(level);
+
+ julLogger.addHandler(
+ new Handler() {
+ @Override
+ public void publish(LogRecord record) {
+ appender.append(record);
+ }
+
+ @Override
+ public void flush() {}
+
+ @Override
+ public void close() throws SecurityException {}
+ });
+ }
+
+ /**
+ * Converts a Log4j Logging Event to a JUL LogRecord
+ *
+ * <p>This is not a full conversion, but covers only the fields we care about. That is the logged
+ * message and eventual thrown Throwables.
+ *
+ * @param event The Log4j LoggingEvent to convert
+ * @return The corresponding JUL LogRecord
+ */
+ public static LogRecord logRecordFromLog4jLoggingEvent(LoggingEvent event) {
+ LogRecord logRecord = new LogRecord(Level.ALL, event.getRenderedMessage());
+ ThrowableInformation tInfo = event.getThrowableInformation();
+ if (tInfo != null) {
+ logRecord.setThrown(tInfo.getThrowable());
+ }
+ return logRecord;
+ }
+
+ /**
+ * Check if a JUL and a Log4j Level correspond
+ *
+ * @param left The JUL Level to check.
+ * @param right The Log4j Level to check.
+ * @return True, if and only if the levels correspond.
+ */
+ public static boolean equalLevels(Level left, org.apache.log4j.Level right) {
+ Level julRight = log4jToJul.get(right);
+ return (left == null && right == null) || (left != null && left.equals(julRight));
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java
new file mode 100644
index 0000000..9a7faa2
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java
@@ -0,0 +1,160 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.googlesource.gerrit.plugins.its.base.util;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase;
+import java.io.IOException;
+import java.math.BigInteger;
+import java.util.HashSet;
+import java.util.Set;
+import org.eclipse.jgit.errors.MissingObjectException;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectLoader;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Test;
+
+public class CommitMessageFetcherTest extends LoggingMockingTestCase {
+ private GitRepositoryManager repoManager;
+ private Repository repo;
+ private String objectIdBlob = "24c5735c3e8ce8fd18d312e9e58149a62236c01a";
+ private String objectIdTree = "3faaefce19558dfc8d9c976f09ae4897f45cb242";
+ private String objectIdCommit = "95aed53c03b6d3df0912bdd9bb1d0c6eaf619f58";
+ private String objectIdMissing = "0123456789012345678901234567890123456789";
+
+ private byte rawBlob[] = "def\n".getBytes();
+ private byte rawTree[] = sha1append("100644 abc\000", objectIdBlob);
+ private byte rawCommit[] =
+ ("tree\0003faaefce19558dfc8d9c976f09ae4897f45cb242\n"
+ + "author Author <author@example.org> 1592579853 +0200\n"
+ + "committer Committer <committer@example.org> 1592579853 +0200\n"
+ + "\n"
+ + "CommitMsg\n")
+ .getBytes();
+
+ private static byte[] sha1append(String left, String sha1sum) {
+ int leftLen = left.length();
+ byte[] right = (new BigInteger(sha1sum, 16)).toByteArray();
+ byte[] ret = new byte[leftLen + right.length];
+ System.arraycopy(left.getBytes(), 0, ret, 0, leftLen);
+ System.arraycopy(right, 0, ret, leftLen, right.length);
+ return ret;
+ }
+
+ @Test
+ public void testFetchBlob() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdBlob);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchTree() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdTree);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchCommit() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdCommit);
+
+ assertThat(commitMessage).isEqualTo("CommitMsg\n");
+ }
+
+ @Test
+ public void testFetchGuardedBlob() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdBlob);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchGuardedTree() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdTree);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchGuardedCommit() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdCommit);
+
+ assertThat(commitMessage).isEqualTo("CommitMsg\n");
+ }
+
+ @Test
+ public void testFetchGuardedMissing() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdMissing);
+
+ assertThat(commitMessage).isEmpty();
+
+ assertLogMessageContains(objectIdMissing);
+ }
+
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ ObjectLoader objectLoaderBlob = mock(ObjectLoader.class);
+ when(objectLoaderBlob.getCachedBytes(anyInt())).thenReturn(rawBlob);
+ when(objectLoaderBlob.getType()).thenReturn(Constants.OBJ_BLOB);
+
+ ObjectLoader objectLoaderTree = mock(ObjectLoader.class);
+ when(objectLoaderTree.getCachedBytes(anyInt())).thenReturn(rawTree);
+ when(objectLoaderTree.getType()).thenReturn(Constants.OBJ_TREE);
+
+ ObjectLoader objectLoaderCommit = mock(ObjectLoader.class);
+ when(objectLoaderCommit.getCachedBytes(anyInt())).thenReturn(rawCommit);
+ when(objectLoaderCommit.getType()).thenReturn(Constants.OBJ_COMMIT);
+
+ Set<ObjectId> shallowCommits = new HashSet<>();
+ shallowCommits.add(ObjectId.fromString(objectIdCommit));
+
+ ObjectReader objectReader = mock(ObjectReader.class);
+ when(objectReader.getShallowCommits()).thenReturn(shallowCommits);
+ when(objectReader.open(ObjectId.fromString(objectIdBlob))).thenReturn(objectLoaderBlob);
+ when(objectReader.open(ObjectId.fromString(objectIdTree))).thenReturn(objectLoaderTree);
+ when(objectReader.open(ObjectId.fromString(objectIdCommit))).thenReturn(objectLoaderCommit);
+ when(objectReader.open(ObjectId.fromString(objectIdMissing)))
+ .thenThrow(
+ new MissingObjectException(ObjectId.fromString(objectIdMissing), Constants.OBJ_COMMIT));
+
+ repo = mock(Repository.class);
+ when(repo.newObjectReader()).thenReturn(objectReader);
+
+ repoManager = mock(GitRepositoryManager.class);
+ when(repoManager.openRepository(eq(Project.nameKey("ProjectFoo")))).thenReturn(repo);
+ }
+
+ private CommitMessageFetcher createCommitMessageFetcher() {
+ return new CommitMessageFetcher(repoManager);
+ }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractorTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractorTest.java
index d4f0605..3a575c7 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractorTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/PropertyExtractorTest.java
@@ -211,6 +211,8 @@
ImmutableMap.of("revision", "testRevision", "patchSetNumber", "3");
when(propertyAttributeExtractor.extractFrom(patchSetAttribute)).thenReturn(patchSetProperties);
+ event.approvals = Suppliers.ofInstance(null);
+
event.comment = "testComment";
changeAttribute.project = "testProject";
changeAttribute.number = 176;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyCommentTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyCommentTest.java
new file mode 100644
index 0000000..8b9e974
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/workflow/AddSoyCommentTest.java
@@ -0,0 +1,196 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.googlesource.gerrit.plugins.its.base.workflow;
+
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.ProvisionException;
+import com.googlesource.gerrit.plugins.its.base.ItsPath;
+import com.googlesource.gerrit.plugins.its.base.its.ItsFacade;
+import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.Map;
+import java.util.UUID;
+import org.eclipse.jgit.util.FileUtils;
+import org.junit.Test;
+
+public class AddSoyCommentTest extends LoggingMockingTestCase {
+ private Injector injector;
+
+ private ItsFacade its;
+ private boolean cleanupSitePath;
+ private Path itsPath;
+
+ @Test
+ public void testNoTemplateFileParameter() throws IOException {
+ ActionRequest actionRequest = new ActionRequest("foo");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, ImmutableMap.of());
+
+ assertLogMessageContains("No template");
+ }
+
+ @Test
+ public void testTemplateFileDoesNotExist() {
+ ActionRequest actionRequest = new ActionRequest("foo nonExistingTemplate");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+
+ assertThrows(
+ ProvisionException.class,
+ () -> addSoyComment.execute(its, "4711", actionRequest, ImmutableMap.of()));
+ }
+
+ @Test
+ public void testPlain() throws IOException {
+ String templateName = "plain";
+
+ ActionRequest actionRequest = new ActionRequest("foo " + templateName);
+
+ injectTemplate(templateName, "bar");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, ImmutableMap.of());
+
+ verify(its).addComment("4711", "bar");
+ }
+
+ @Test
+ public void testParameterPlain() throws IOException {
+ String templateName = "parameterPlain";
+
+ ActionRequest actionRequest = new ActionRequest("foo " + templateName);
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder().put("param1", "simple").build();
+
+ injectTemplate(templateName, "{@param param1:string}{$param1}");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, properties);
+
+ verify(its).addComment("4711", "simple");
+ }
+
+ @Test
+ public void testParameterEscapingDefault() throws IOException {
+ String templateName = "parameterEscapingDefault";
+
+ ActionRequest actionRequest = new ActionRequest("foo " + templateName);
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder().put("param1", "<b>bar\"/>").build();
+
+ injectTemplate(templateName, "{@param param1:string}{$param1}");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, properties);
+
+ verify(its).addComment("4711", "<b>bar"/>");
+ }
+
+ @Test
+ public void testParameterEscapingTextUnescaped() throws IOException {
+ String templateName = "parameterEscapingTextUnescaped";
+
+ ActionRequest actionRequest = new ActionRequest("foo " + templateName);
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder().put("param1", "<b>bar\"/>").build();
+
+ injectTemplate(templateName, "text", "{@param param1:string}{$param1}");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, properties);
+
+ verify(its).addComment("4711", "<b>bar\"/>");
+ }
+
+ @Test
+ public void testParameterEscapingTextForcedEscapingUri() throws IOException {
+ String templateName = "parameterEscapingTextForcedEscapingUri";
+
+ ActionRequest actionRequest = new ActionRequest("foo " + templateName);
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder().put("param1", "<b>bar\"/>").build();
+
+ injectTemplate(templateName, "text", "{@param param1:string}{$param1|escapeUri}");
+
+ AddSoyComment addSoyComment = createAddSoyComment();
+ addSoyComment.execute(its, "4711", actionRequest, properties);
+
+ verify(its).addComment("4711", "%3Cb%3Ebar%22%2F%3E");
+ }
+
+ private AddSoyComment createAddSoyComment() {
+ return injector.getInstance(AddSoyComment.class);
+ }
+
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+ cleanupSitePath = false;
+ injector = Guice.createInjector(new TestModule());
+ }
+
+ @Override
+ public void tearDown() throws Exception {
+ if (cleanupSitePath) {
+ if (Files.exists(itsPath)) {
+ FileUtils.delete(itsPath.toFile(), FileUtils.RECURSIVE);
+ }
+ }
+ super.tearDown();
+ }
+
+ private void injectTemplate(String name, String content) throws IOException {
+ injectTemplate(name, null, content);
+ }
+
+ private void injectTemplate(String name, String kind, String content) throws IOException {
+ Path templatePath = itsPath.resolve("templates").resolve(name + ".soy");
+ Files.createDirectories(templatePath.getParent());
+ String namespace = "{namespace etc.its.templates}";
+ String opening = "{template ." + name + (kind != null ? (" kind=\"" + kind + "\"") : "") + "}";
+ String closing = "{/template}";
+
+ String fullTemplate = namespace + opening + content + closing;
+
+ Files.write(templatePath, fullTemplate.getBytes());
+ }
+
+ private Path randomTargetPath() {
+ return Paths.get("target", "random_name_" + UUID.randomUUID().toString().replaceAll("-", "_"));
+ }
+
+ private class TestModule extends FactoryModule {
+ @Override
+ protected void configure() {
+ its = mock(ItsFacade.class);
+ bind(ItsFacade.class).toInstance(its);
+
+ itsPath = randomTargetPath().resolve("etc").resolve("its");
+ assertFalse("itsPath (" + itsPath + ") already exists", Files.exists(itsPath));
+ cleanupSitePath = true;
+ bind(Path.class).annotatedWith(ItsPath.class).toInstance(itsPath);
+ }
+ }
+}
