Merge changes I83e43867,I942b17c8,Ia039afc7
* changes:
ProjectConfig: Consistently use String.format to format error messages
ProjectConfig: Simplify error reporting
ProjectConfig: Remove unused constant
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 513aeed..975ad52 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -125,7 +125,6 @@
public static final String KEY_BRANCH = "branch";
public static final String SUBMIT_REQUIREMENT = "submit-requirement";
- public static final String KEY_SR_NAME = "name";
public static final String KEY_SR_DESCRIPTION = "description";
public static final String KEY_SR_APPLICABILITY_EXPRESSION = "applicableIf";
public static final String KEY_SR_SUBMITTABILITY_EXPRESSION = "submittableIf";
@@ -643,7 +642,7 @@
if (rc.getStringList(ACCESS, null, KEY_INHERIT_FROM).length > 1) {
// The config must not contain more than one parent to inherit from
// as there is no guarantee which of the parents would be used then.
- error(ValidationError.create(PROJECT_CONFIG, "Cannot inherit from multiple projects"));
+ error("Cannot inherit from multiple projects");
}
p.setParent(rc.getString(ACCESS, null, KEY_INHERIT_FROM));
@@ -696,10 +695,8 @@
String lower = name.toLowerCase();
if (lowerNames.containsKey(lower)) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Extension Panels \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower))));
+ String.format(
+ "Extension Panels \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower)));
}
lowerNames.put(lower, name);
extensionPanelSections.put(
@@ -725,26 +722,14 @@
ca.setAutoVerify(null);
} else if (rules.size() > 1) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- "Invalid rule in "
- + CONTRIBUTOR_AGREEMENT
- + "."
- + name
- + "."
- + KEY_AUTO_VERIFY
- + ": at most one group may be set"));
+ String.format(
+ "Invalid rule in %s.%s.%s: at most one group may be set",
+ CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY));
} else if (rules.get(0).getAction() != Action.ALLOW) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- "Invalid rule in "
- + CONTRIBUTOR_AGREEMENT
- + "."
- + name
- + "."
- + KEY_AUTO_VERIFY
- + ": the group must be allowed"));
+ String.format(
+ "Invalid rule in %s.%s.%s: the group must be allowed",
+ CONTRIBUTOR_AGREEMENT, name, KEY_AUTO_VERIFY));
} else {
ca.setAutoVerify(rules.get(0).getGroup());
}
@@ -792,21 +777,16 @@
if (ref.getUUID() != null) {
n.addGroup(ref);
} else {
- error(
- ValidationError.create(
- PROJECT_CONFIG,
- "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME));
+ error(String.format("group \"%s\" not in %s", ref.getName(), GroupList.FILE_NAME));
}
} else if (dst.startsWith("user ")) {
- error(ValidationError.create(PROJECT_CONFIG, dst + " not supported"));
+ error(String.format("%s not supported", dst));
} else {
try {
n.addAddress(Address.parse(dst));
} catch (IllegalArgumentException err) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- "notify section \"" + sectionName + "\" has invalid email \"" + dst + "\""));
+ String.format("notify section \"%s\" has invalid email \"%s\"", sectionName, dst));
}
}
}
@@ -867,7 +847,7 @@
try {
RefPattern.validateRegExp(refPattern);
} catch (InvalidNameException e) {
- error(ValidationError.create(PROJECT_CONFIG, "Invalid ref name: " + e.getMessage()));
+ error(String.format("Invalid ref name: %s", e.getMessage()));
return false;
}
return true;
@@ -895,9 +875,7 @@
// to fail fast if any of the patterns are invalid.
patterns.add(Pattern.compile(patternString).pattern());
} catch (PatternSyntaxException e) {
- error(
- ValidationError.create(
- PROJECT_CONFIG, "Invalid regular expression: " + e.getMessage()));
+ error(String.format("Invalid regular expression: %s", e.getMessage()));
continue;
}
}
@@ -924,15 +902,11 @@
rule = PermissionRule.fromString(ruleString, useRange);
} catch (IllegalArgumentException notRule) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- "Invalid rule in "
- + section
- + (subsection != null ? "." + subsection : "")
- + "."
- + varName
- + ": "
- + notRule.getMessage()));
+ String.format(
+ "Invalid rule in %s.%s: %s",
+ section + (subsection != null ? "." + subsection : ""),
+ varName,
+ notRule.getMessage()));
continue;
}
@@ -943,9 +917,7 @@
// all rules in the same file share the same GroupReference.
//
ref = groupList.resolve(rule.getGroup());
- error(
- ValidationError.create(
- PROJECT_CONFIG, "group \"" + ref.getName() + "\" not in " + GroupList.FILE_NAME));
+ error(String.format("group \"%s\" not in %s", ref.getName(), GroupList.FILE_NAME));
}
perm.add(rule.toBuilder().setGroup(ref));
@@ -970,11 +942,9 @@
String lower = name.toLowerCase();
if (lowerNames.containsKey(lower)) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Submit requirement \"%s\" conflicts with \"%s\". Skipping the former.",
- name, lowerNames.get(lower))));
+ String.format(
+ "Submit requirement \"%s\" conflicts with \"%s\". Skipping the former.",
+ name, lowerNames.get(lower)));
continue;
}
lowerNames.put(lower, name);
@@ -987,12 +957,10 @@
if (blockExpr == null) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- (String.format(
- "Submit requirement \"%s\" does not define a submittability expression."
- + " Skipping this requirement.",
- name))));
+ String.format(
+ "Submit requirement \"%s\" does not define a submittability expression."
+ + " Skipping this requirement.",
+ name));
continue;
}
@@ -1019,10 +987,7 @@
for (String name : rc.getSubsections(LABEL)) {
String lower = name.toLowerCase();
if (lowerNames.containsKey(lower)) {
- error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format("Label \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower))));
+ error(String.format("Label \"%s\" conflicts with \"%s\"", name, lowerNames.get(lower)));
}
lowerNames.put(lower, name);
@@ -1034,18 +999,13 @@
if (allValues.add(labelValue.getValue())) {
values.add(labelValue);
} else {
- error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format("Duplicate %s \"%s\" for label \"%s\"", KEY_VALUE, value, name)));
+ error(String.format("Duplicate %s \"%s\" for label \"%s\"", KEY_VALUE, value, name));
}
} catch (IllegalArgumentException notValue) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Invalid %s \"%s\" for label \"%s\": %s",
- KEY_VALUE, value, name, notValue.getMessage())));
+ String.format(
+ "Invalid %s \"%s\" for label \"%s\": %s",
+ KEY_VALUE, value, name, notValue.getMessage()));
}
}
@@ -1053,7 +1013,7 @@
try {
label = LabelType.builder(name, values);
} catch (IllegalArgumentException badName) {
- error(ValidationError.create(PROJECT_CONFIG, String.format("Invalid label \"%s\"", name)));
+ error(String.format("Invalid label \"%s\"", name));
continue;
}
@@ -1064,11 +1024,9 @@
: Optional.of(LabelFunction.MAX_WITH_BLOCK);
if (!function.isPresent()) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Invalid %s for label \"%s\". Valid names are: %s",
- KEY_FUNCTION, name, Joiner.on(", ").join(LabelFunction.ALL.keySet()))));
+ String.format(
+ "Invalid %s for label \"%s\". Valid names are: %s",
+ KEY_FUNCTION, name, Joiner.on(", ").join(LabelFunction.ALL.keySet())));
}
label.setFunction(function.orElse(null));
label.setCopyCondition(rc.getString(LABEL, name, KEY_COPY_CONDITION));
@@ -1078,11 +1036,7 @@
if (isInRange(dv, values)) {
label.setDefaultValue(dv);
} else {
- error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Invalid %s \"%s\" for label \"%s\"", KEY_DEFAULT_VALUE, dv, name)));
+ error(String.format("Invalid %s \"%s\" for label \"%s\"", KEY_DEFAULT_VALUE, dv, name));
}
}
label.setAllowPostSubmit(
@@ -1131,18 +1085,13 @@
short copyValue = Shorts.checkedCast(PermissionRule.parseInt(value));
if (!copyValues.add(copyValue)) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Duplicate %s \"%s\" for label \"%s\"", KEY_COPY_VALUE, value, name)));
+ String.format("Duplicate %s \"%s\" for label \"%s\"", KEY_COPY_VALUE, value, name));
}
} catch (IllegalArgumentException notValue) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Invalid %s \"%s\" for label \"%s\": %s",
- KEY_COPY_VALUE, value, name, notValue.getMessage())));
+ String.format(
+ "Invalid %s \"%s\" for label \"%s\": %s",
+ KEY_COPY_VALUE, value, name, notValue.getMessage()));
}
}
label.setCopyValues(copyValues);
@@ -1177,18 +1126,14 @@
commentLinkSections.put(name, buildCommentLink(rc, name, false));
} catch (PatternSyntaxException e) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Invalid pattern \"%s\" in commentlink.%s.match: %s",
- rc.getString(COMMENTLINK, name, KEY_MATCH), name, e.getMessage())));
+ String.format(
+ "Invalid pattern \"%s\" in commentlink.%s.match: %s",
+ rc.getString(COMMENTLINK, name, KEY_MATCH), name, e.getMessage()));
} catch (IllegalArgumentException e) {
error(
- ValidationError.create(
- PROJECT_CONFIG,
- String.format(
- "Error in pattern \"%s\" in commentlink.%s.match: %s",
- rc.getString(COMMENTLINK, name, KEY_MATCH), name, e.getMessage())));
+ String.format(
+ "Error in pattern \"%s\" in commentlink.%s.match: %s",
+ rc.getString(COMMENTLINK, name, KEY_MATCH), name, e.getMessage()));
}
}
}
@@ -1230,9 +1175,7 @@
if (groupName != null) {
GroupReference ref = groupList.byName(groupName);
if (ref == null) {
- error(
- ValidationError.create(
- PROJECT_CONFIG, "group \"" + groupName + "\" not in " + GroupList.FILE_NAME));
+ error(String.format("group \"%s\" not in %s", groupName, GroupList.FILE_NAME));
}
rc.setString(PLUGIN, plugin, name, value);
}
@@ -1782,11 +1725,15 @@
try {
return rc.getEnum(section, subsection, name, defaultValue);
} catch (IllegalArgumentException err) {
- error(ValidationError.create(PROJECT_CONFIG, err.getMessage()));
+ error(err.getMessage());
return defaultValue;
}
}
+ private void error(String errorMessage) {
+ error(ValidationError.create(PROJECT_CONFIG, errorMessage));
+ }
+
@Override
public void error(ValidationError error) {
if (validationErrors == null) {
