Include plugin name into validation messages
This way it's clear from which plugin these messages are coming. E.g. if
hosts have both, the code-owners plugin and the find-owners plugin,
installed the messages can be confusing if that's not clear (e.g. if the
find-owners plugin reports validation issues and the code-owners plugin
is configured to not do validation and says that the validation is
skipped).
Change-Id: I7311efb44205deb9bfb34af6528294ca82e653f7
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
index 36cf83c..e4714bd 100644
--- a/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
+++ b/java/com/google/gerrit/plugins/codeowners/validation/CodeOwnerConfigValidator.java
@@ -29,6 +29,7 @@
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.plugins.codeowners.backend.ChangedFiles;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfig;
@@ -125,6 +126,7 @@
public class CodeOwnerConfigValidator implements CommitValidationListener, MergeValidationListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ private final String pluginName;
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
private final GitRepositoryManager repoManager;
private final ChangedFiles changedFiles;
@@ -137,6 +139,7 @@
@Inject
CodeOwnerConfigValidator(
+ @PluginName String pluginName,
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
GitRepositoryManager repoManager,
ChangedFiles changedFiles,
@@ -146,6 +149,7 @@
ChangeNotes.Factory changeNotesFactory,
PatchSetUtil patchSetUtil,
IdentifiedUser.GenericFactory userFactory) {
+ this.pluginName = pluginName;
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
this.repoManager = repoManager;
this.changedFiles = changedFiles;
@@ -178,6 +182,7 @@
validationResult =
Optional.of(
ValidationResult.create(
+ pluginName,
"skipping validation of code owner config files",
new CommitValidationMessage(
"code owners config validation is disabled", ValidationMessage.Type.HINT)));
@@ -246,6 +251,7 @@
validationResult =
Optional.of(
ValidationResult.create(
+ pluginName,
"skipping validation of code owner config files",
new CommitValidationMessage(
"code owners config validation is disabled", ValidationMessage.Type.HINT)));
@@ -300,6 +306,7 @@
if (codeOwnersPluginConfiguration.isDisabled(branchNameKey)) {
return Optional.of(
ValidationResult.create(
+ pluginName,
"skipping validation of code owner config files",
new CommitValidationMessage(
"code-owners functionality is disabled", ValidationMessage.Type.HINT)));
@@ -307,6 +314,7 @@
if (codeOwnersPluginConfiguration.areCodeOwnerConfigsReadOnly(branchNameKey.project())) {
return Optional.of(
ValidationResult.create(
+ pluginName,
"modifying code owner config files not allowed",
new CommitValidationMessage(
"code owner config files are configured to be read-only",
@@ -352,6 +360,7 @@
// validate the code owner config files
return Optional.of(
ValidationResult.create(
+ pluginName,
modifiedCodeOwnerConfigFiles.stream()
.flatMap(
changedFile ->
@@ -375,6 +384,7 @@
e.getMessage()));
return Optional.of(
ValidationResult.create(
+ pluginName,
"skipping validation of code owner config files",
new CommitValidationMessage(
"code-owners plugin configuration is invalid,"
@@ -1011,21 +1021,26 @@
"code owner config files validated, no issues found";
private static final String INVALID_MSG = "invalid code owner config files";
+ abstract String pluginName();
+
abstract String summaryMessage();
abstract ImmutableList<CommitValidationMessage> validationMessages();
static ValidationResult create(
- String summaryMessage, CommitValidationMessage commitValidationMessage) {
+ String pluginName, String summaryMessage, CommitValidationMessage commitValidationMessage) {
return new AutoValue_CodeOwnerConfigValidator_ValidationResult(
- summaryMessage, ImmutableList.of(commitValidationMessage));
+ pluginName, summaryMessage, ImmutableList.of(commitValidationMessage));
}
- static ValidationResult create(Stream<CommitValidationMessage> validationMessagesStream) {
+ static ValidationResult create(
+ String pluginName, Stream<CommitValidationMessage> validationMessagesStream) {
ImmutableList<CommitValidationMessage> validationMessages =
validationMessagesStream.collect(toImmutableList());
return new AutoValue_CodeOwnerConfigValidator_ValidationResult(
- validationMessages.isEmpty() ? NO_ISSUES_MSG : INVALID_MSG, validationMessages);
+ pluginName,
+ validationMessages.isEmpty() ? NO_ISSUES_MSG : INVALID_MSG,
+ validationMessages);
}
/**
@@ -1040,7 +1055,8 @@
List<CommitValidationMessage> processForOnCommitReceived(boolean dryRun)
throws CommitValidationException {
if (!dryRun && hasError()) {
- throw new CommitValidationException(summaryMessage(), validationMessages());
+ throw new CommitValidationException(
+ withPluginName(summaryMessage()), withPluginName(validationMessages()));
}
return validationMessagesWithIncludedSummaryMessage();
@@ -1081,8 +1097,8 @@
return ImmutableList.<CommitValidationMessage>builder()
.add(
new CommitValidationMessage(
- summaryMessage(), getValidationMessageTypeForSummaryMessage()))
- .addAll(validationMessages())
+ withPluginName(summaryMessage()), getValidationMessageTypeForSummaryMessage()))
+ .addAll(withPluginName(validationMessages()))
.build();
}
@@ -1130,7 +1146,7 @@
*/
private String getMessage(List<CommitValidationMessage> validationMessages) {
checkState(!validationMessages.isEmpty(), "expected at least 1 validation message");
- StringBuilder msgBuilder = new StringBuilder(summaryMessage()).append(":");
+ StringBuilder msgBuilder = new StringBuilder(withPluginName(summaryMessage())).append(":");
for (CommitValidationMessage msg : validationMessages) {
msgBuilder
.append("\n ")
@@ -1140,5 +1156,16 @@
}
return msgBuilder.toString();
}
+
+ private String withPluginName(String message) {
+ return "[" + pluginName() + "] " + message;
+ }
+
+ private ImmutableList<CommitValidationMessage> withPluginName(
+ ImmutableList<CommitValidationMessage> validationMessages) {
+ return validationMessages.stream()
+ .map(msg -> new CommitValidationMessage(withPluginName(msg.getMessage()), msg.getType()))
+ .collect(toImmutableList());
+ }
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
index 9c8207a..e65efbd 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/CodeOwnerConfigValidatorIT.java
@@ -469,7 +469,7 @@
.isEqualTo(
String.format(
"Failed to submit 1 change due to the following problems:\n"
- + "Change %d: invalid code owner config files:\n"
+ + "Change %d: [code-owners] invalid code owner config files:\n"
+ " ERROR: code owner email '%s' in '%s' cannot be resolved for %s",
r.getChange().getId().get(),
unknownEmail,
@@ -807,10 +807,11 @@
String abbreviatedCommit = abbreviateName(r.getCommit());
r.assertErrorStatus(
- String.format("commit %s: %s", abbreviatedCommit, "invalid code owner config files"));
+ String.format(
+ "commit %s: [code-owners] %s", abbreviatedCommit, "invalid code owner config files"));
r.assertMessage(
String.format(
- "error: commit %s: %s",
+ "error: commit %s: [code-owners] %s",
abbreviatedCommit,
String.format(
"code owner email '%s' in '%s' cannot be resolved for %s",
@@ -821,7 +822,7 @@
// the pre-existing issue is returned as warning
r.assertMessage(
String.format(
- "warning: commit %s: code owner email '%s' in '%s' cannot be resolved for %s",
+ "warning: commit %s: [code-owners] code owner email '%s' in '%s' cannot be resolved for %s",
abbreviatedCommit,
unknownEmail1,
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath(),
@@ -866,7 +867,7 @@
.isEqualTo(
String.format(
"Failed to submit 1 change due to the following problems:\n"
- + "Change %d: invalid code owner config files:\n"
+ + "Change %d: [code-owners] invalid code owner config files:\n"
+ " ERROR: code owner email '%s' in '%s' cannot be resolved for %s",
r.getChange().getId().get(),
unknownEmail,
@@ -915,7 +916,7 @@
r.assertOkStatus();
r.assertMessage(
String.format(
- "error: commit %s: %s",
+ "error: commit %s: [code-owners] %s",
abbreviatedCommit,
String.format(
"code owner email '%s' in '%s' cannot be resolved for %s",
@@ -926,7 +927,8 @@
// the pre-existing issue is returned as warning
r.assertMessage(
String.format(
- "warning: commit %s: code owner email '%s' in '%s' cannot be resolved for %s",
+ "warning: commit %s: [code-owners] code owner email '%s' in '%s' cannot be resolved"
+ + " for %s",
abbreviatedCommit,
unknownEmail1,
codeOwnerConfigOperations.codeOwnerConfig(codeOwnerConfigKey).getFilePath(),
@@ -979,7 +981,7 @@
.isEqualTo(
String.format(
"Failed to submit 1 change due to the following problems:\n"
- + "Change %d: invalid code owner config files:\n"
+ + "Change %d: [code-owners] invalid code owner config files:\n"
+ " ERROR: code owner email '%s' in '%s' cannot be resolved for %s",
r.getChange().getId().get(),
admin.email(),
@@ -1791,7 +1793,8 @@
pushResult.assertOkStatus();
for (String hint : hints) {
pushResult.assertMessage(
- String.format("hint: commit %s: %s", abbreviateName(pushResult.getCommit()), hint));
+ String.format(
+ "hint: commit %s: [code-owners] %s", abbreviateName(pushResult.getCommit()), hint));
}
pushResult.assertNotMessage("fatal");
pushResult.assertNotMessage("error");
@@ -1803,7 +1806,8 @@
pushResult.assertOkStatus();
for (String error : errors) {
pushResult.assertMessage(
- String.format("fatal: commit %s: %s", abbreviateName(pushResult.getCommit()), error));
+ String.format(
+ "fatal: commit %s: [code-owners] %s", abbreviateName(pushResult.getCommit()), error));
}
pushResult.assertNotMessage("error");
pushResult.assertNotMessage("warning");
@@ -1815,7 +1819,8 @@
pushResult.assertOkStatus();
for (String error : errors) {
pushResult.assertMessage(
- String.format("error: commit %s: %s", abbreviateName(pushResult.getCommit()), error));
+ String.format(
+ "error: commit %s: [code-owners] %s", abbreviateName(pushResult.getCommit()), error));
}
pushResult.assertNotMessage("fatal");
pushResult.assertNotMessage("warning");
@@ -1827,7 +1832,9 @@
pushResult.assertOkStatus();
for (String warning : warnings) {
pushResult.assertMessage(
- String.format("warning: commit %s: %s", abbreviateName(pushResult.getCommit()), warning));
+ String.format(
+ "warning: commit %s: [code-owners] %s",
+ abbreviateName(pushResult.getCommit()), warning));
}
pushResult.assertNotMessage("fatal");
pushResult.assertNotMessage("error");
@@ -1837,9 +1844,11 @@
private void assertErrorWithMessages(
PushOneCommit.Result pushResult, String summaryMessage, String... errors) throws Exception {
String abbreviatedCommit = abbreviateName(pushResult.getCommit());
- pushResult.assertErrorStatus(String.format("commit %s: %s", abbreviatedCommit, summaryMessage));
+ pushResult.assertErrorStatus(
+ String.format("commit %s: [code-owners] %s", abbreviatedCommit, summaryMessage));
for (String error : errors) {
- pushResult.assertMessage(String.format("error: commit %s: %s", abbreviatedCommit, error));
+ pushResult.assertMessage(
+ String.format("error: commit %s: [code-owners] %s", abbreviatedCommit, error));
}
pushResult.assertNotMessage("fatal");
pushResult.assertNotMessage("warning");
@@ -1849,9 +1858,11 @@
private void assertFatalWithMessages(
PushOneCommit.Result pushResult, String summaryMessage, String... errors) throws Exception {
String abbreviatedCommit = abbreviateName(pushResult.getCommit());
- pushResult.assertErrorStatus(String.format("commit %s: %s", abbreviatedCommit, summaryMessage));
+ pushResult.assertErrorStatus(
+ String.format("commit %s: [code-owners] %s", abbreviatedCommit, summaryMessage));
for (String error : errors) {
- pushResult.assertMessage(String.format("fatal: commit %s: %s", abbreviatedCommit, error));
+ pushResult.assertMessage(
+ String.format("fatal: commit %s: [code-owners] %s", abbreviatedCommit, error));
}
pushResult.assertNotMessage("error");
pushResult.assertNotMessage("warning");
