Add ValidationMessage.Type

This allows us to distinguish errors, warnings, hints and other
messages.

Since the highlighting in git-core wants to see the marker keyword
("ERROR") at the start of the line, we have to put the message type in
ValidationMessage, so we output

  WARNING: commit abcdef: bla bla

rather than

  commit abcdef: WARNING: bla bla

Change-Id: I95e7298428830f5fa1a40a9cac2015e0ea482dc7
This commit is contained in:
Han-Wen Nienhuys
2018-10-01 19:07:59 +02:00
parent 79b545e4d0
commit bef019fa09
5 changed files with 57 additions and 20 deletions

View File

@@ -110,7 +110,7 @@ public class BranchCommitValidator {
for (CommitValidationMessage m : validators.validate(receiveEvent)) { for (CommitValidationMessage m : validators.validate(receiveEvent)) {
messages.add( messages.add(
new CommitValidationMessage(messageForCommit(commit, m.getMessage()), m.isError())); new CommitValidationMessage(messageForCommit(commit, m.getMessage()), m.getType()));
} }
} catch (CommitValidationException e) { } catch (CommitValidationException e) {
logger.atFine().log("Commit validation failed on %s", commit.name()); logger.atFine().log("Commit validation failed on %s", commit.name());
@@ -118,7 +118,7 @@ public class BranchCommitValidator {
// The non-error messages may contain background explanation for the // The non-error messages may contain background explanation for the
// fatal error, so have to preserve all messages. // fatal error, so have to preserve all messages.
messages.add( messages.add(
new CommitValidationMessage(messageForCommit(commit, m.getMessage()), m.isError())); new CommitValidationMessage(messageForCommit(commit, m.getMessage()), m.getType()));
} }
cmd.setResult(REJECTED_OTHER_REASON, messageForCommit(commit, e.getMessage())); cmd.setResult(REJECTED_OTHER_REASON, messageForCommit(commit, e.getMessage()));
return false; return false;

View File

@@ -481,20 +481,26 @@ class ReceiveCommits {
return project; return project;
} }
private void addMessage(String message, ValidationMessage.Type type) {
messages.add(new CommitValidationMessage(message, type));
}
private void addMessage(String message) { private void addMessage(String message) {
messages.add(new CommitValidationMessage(message, false)); messages.add(new CommitValidationMessage(message, ValidationMessage.Type.OTHER));
} }
private void addError(String error) { private void addError(String error) {
messages.add(new CommitValidationMessage(error, true)); addMessage(error, ValidationMessage.Type.ERROR);
} }
void sendMessages() { void sendMessages() {
for (ValidationMessage m : messages) { for (ValidationMessage m : messages) {
String msg = m.getType().getPrefix() + m.getMessage();
if (m.isError()) { if (m.isError()) {
messageSender.sendError(m.getMessage()); messageSender.sendError(msg);
} else { } else {
messageSender.sendMessage(m.getMessage()); messageSender.sendMessage(msg);
} }
} }
} }
@@ -2261,11 +2267,8 @@ class ReceiveCommits {
rw.parseBody(c); rw.parseBody(c);
messages.add( messages.add(
new CommitValidationMessage( new CommitValidationMessage(
"ERROR: Implicit Merge of " "Implicit Merge of " + c.abbreviate(7).name() + " " + c.getShortMessage(),
+ c.abbreviate(7).name() ValidationMessage.Type.ERROR));
+ " "
+ c.getShortMessage(),
false));
} }
reject(magicBranch.cmd, "implicit merges detected"); reject(magicBranch.cmd, "implicit merges detected");
} }

View File

@@ -15,6 +15,10 @@
package com.google.gerrit.server.git.validators; package com.google.gerrit.server.git.validators;
public class CommitValidationMessage extends ValidationMessage { public class CommitValidationMessage extends ValidationMessage {
public CommitValidationMessage(String message, ValidationMessage.Type type) {
super(message, type);
}
public CommitValidationMessage(String message, boolean isError) { public CommitValidationMessage(String message, boolean isError) {
super(message, isError); super(message, isError);
} }

View File

@@ -45,6 +45,7 @@ import com.google.gerrit.server.config.UrlFormatter;
import com.google.gerrit.server.events.CommitReceivedEvent; import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ValidationError; import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.validators.ValidationMessage.Type;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.permissions.RefPermission;
@@ -310,7 +311,7 @@ public class CommitValidators {
private CommitValidationMessage getMissingChangeIdErrorMsg(String errMsg, RevCommit c) { private CommitValidationMessage getMissingChangeIdErrorMsg(String errMsg, RevCommit c) {
StringBuilder sb = new StringBuilder(); StringBuilder sb = new StringBuilder();
sb.append("ERROR: ").append(errMsg).append("\n"); sb.append(errMsg).append("\n");
boolean hinted = false; boolean hinted = false;
if (c.getFullMessage().contains(CHANGE_ID_PREFIX)) { if (c.getFullMessage().contains(CHANGE_ID_PREFIX)) {
@@ -332,7 +333,7 @@ public class CommitValidators {
.append("and then amend the commit:\n") .append("and then amend the commit:\n")
.append(" git commit --amend\n"); .append(" git commit --amend\n");
} }
return new CommitValidationMessage(sb.toString(), false); return new CommitValidationMessage(sb.toString(), Type.ERROR);
} }
private String getCommitMessageHookInstallationHint() { private String getCommitMessageHookInstallationHint() {
@@ -692,7 +693,10 @@ public class CommitValidators {
.map( .map(
p -> p ->
new CommitValidationMessage( new CommitValidationMessage(
p.message, p.status == ConsistencyProblemInfo.Status.ERROR)) p.message,
p.status == ConsistencyProblemInfo.Status.ERROR
? ValidationMessage.Type.ERROR
: ValidationMessage.Type.OTHER))
.collect(toList()); .collect(toList());
if (msgs.stream().anyMatch(ValidationMessage::isError)) { if (msgs.stream().anyMatch(ValidationMessage::isError)) {
throw new CommitValidationException("invalid external IDs", msgs); throw new CommitValidationException("invalid external IDs", msgs);
@@ -753,7 +757,7 @@ public class CommitValidators {
"invalid account configuration", "invalid account configuration",
errorMessages errorMessages
.stream() .stream()
.map(m -> new CommitValidationMessage(m, true)) .map(m -> new CommitValidationMessage(m, Type.ERROR))
.collect(toList())); .collect(toList()));
} }
} catch (IOException e) { } catch (IOException e) {
@@ -836,7 +840,7 @@ public class CommitValidators {
.append(urlFormatter.getSettingsUrl("EmailAddresses").get()) .append(urlFormatter.getSettingsUrl("EmailAddresses").get())
.append("\n\n"); .append("\n\n");
} }
return new CommitValidationMessage(sb.toString(), true); return new CommitValidationMessage(sb.toString(), Type.ERROR);
} }
/** /**
@@ -857,6 +861,6 @@ public class CommitValidators {
} }
private static void addError(String error, List<CommitValidationMessage> messages) { private static void addError(String error, List<CommitValidationMessage> messages) {
messages.add(new CommitValidationMessage(error, true)); messages.add(new CommitValidationMessage(error, Type.ERROR));
} }
} }

View File

@@ -15,19 +15,45 @@
package com.google.gerrit.server.git.validators; package com.google.gerrit.server.git.validators;
public class ValidationMessage { public class ValidationMessage {
public enum Type {
ERROR("ERROR: "),
WARNING("WARNING: "),
HINT("hint: "),
OTHER("");
private final String prefix;
Type(String prefix) {
this.prefix = prefix;
}
public String getPrefix() {
return prefix;
}
};
private final String message; private final String message;
private final boolean isError; private final Type type;
public ValidationMessage(String message, Type type) {
this.message = message;
this.type = type;
}
public ValidationMessage(String message, boolean isError) { public ValidationMessage(String message, boolean isError) {
this.message = message; this.message = message;
this.isError = isError; this.type = (isError ? Type.ERROR : Type.OTHER);
} }
public String getMessage() { public String getMessage() {
return message; return message;
} }
public Type getType() {
return type;
}
public boolean isError() { public boolean isError() {
return isError; return type == Type.ERROR;
} }
} }