GetCodeOwnerProjectConfig should not fail with 500 ISE if config is invalid
If the plugin configuration is invalid we want to fail with 409 Conflict
rather than 500 Internal Server Error. This is ensured by
InvalidPluginConfigurationException.ExceptionHook which returns
409 Conflict if an exception is caused by
InvalidPluginConfigurationException, but there was one case where an
invalid configuration didn't throw this exception and hence the request
failed with 500 ISE. This was the case if the default required approval
(Code-Review+1) was used but the project didn't have the Code-Review
label.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I76e6de49b089f211a03a81c52430296806b8fe5d
diff --git a/java/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfig.java b/java/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfig.java
index 3511f46..dcf50e7 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfig.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/AbstractRequiredApprovalConfig.java
@@ -38,7 +38,8 @@
* parent projects.
*/
abstract class AbstractRequiredApprovalConfig {
- private final String pluginName;
+ protected final String pluginName;
+
private final PluginConfigFactory pluginConfigFactory;
AbstractRequiredApprovalConfig(
diff --git a/java/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfig.java b/java/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfig.java
index 66b6cc8..e49904d 100644
--- a/java/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfig.java
+++ b/java/com/google/gerrit/plugins/codeowners/config/RequiredApprovalConfig.java
@@ -14,6 +14,8 @@
package com.google.gerrit.plugins.codeowners.config;
+import static com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS;
+
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.server.config.PluginConfigFactory;
@@ -54,6 +56,21 @@
}
RequiredApproval createDefault(ProjectState projectState) throws IllegalStateException {
- return RequiredApproval.createDefault(projectState, DEFAULT_LABEL, DEFAULT_VALUE);
+ try {
+ return RequiredApproval.createDefault(projectState, DEFAULT_LABEL, DEFAULT_VALUE);
+ } catch (IllegalStateException | IllegalArgumentException e) {
+ throw new InvalidPluginConfigurationException(
+ pluginName,
+ String.format(
+ "The default required approval '%s+%d' that is used for project %s is not valid: %s"
+ + " Please configure a valid required approval in %s.config (parameter %s.%s).",
+ DEFAULT_LABEL,
+ DEFAULT_VALUE,
+ projectState.getName(),
+ e.getMessage(),
+ pluginName,
+ SECTION_CODE_OWNERS,
+ getConfigKey()));
+ }
}
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerProjectConfigRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerProjectConfigRestIT.java
index 1ad14ce..a39cab8 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerProjectConfigRestIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerProjectConfigRestIT.java
@@ -51,4 +51,26 @@
+ " 'non-existing-backend' that is configured in gerrit.config (parameter"
+ " plugin.code-owners.backend) not found.");
}
+
+ @Test
+ public void cannotGetStatusIfPluginConfigurationIsInvalid_defaultRequiredApprovalConfigIsInvalid()
+ throws Exception {
+ // Delete the Code-Review label which is required as code owner approval by default.
+ gApi.projects().name(allProjects.get()).label("Code-Review").delete();
+
+ RestResponse r =
+ adminRestSession.get(
+ String.format(
+ "/projects/%s/code_owners.project_config", IdString.fromDecoded(project.get())));
+ r.assertConflict();
+ assertThat(r.getEntityContent())
+ .contains(
+ String.format(
+ "Invalid configuration of the code-owners plugin. The default required approval"
+ + " 'Code-Review+1' that is used for project %s is not valid:"
+ + " Default label Code-Review doesn't exist for project %s."
+ + " Please configure a valid required approval in code-owners.config"
+ + " (parameter codeOwners.requiredApproval).",
+ project.get(), project.get()));
+ }
}
