Allow to set required approval and override approval via REST This makes changing the approval configuration easier. Signed-off-by: Edwin Kempin <ekempin@google.com> Change-Id: Ic5883790b932391440e099b7a20bfc9e2817ea4b
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java index 5d899f3..684d715 100644 --- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java +++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java
@@ -40,4 +40,25 @@ /** The file extension that should be used for code owner config files in this project. */ public String fileExtension; + + /** + * The approval that is required from code owners. + * + * <p>The required approval must be specified in the format {@code <label-name>+<label-value>}. + * + * <p>If an empty string is provided the required approval configuration is unset. Unsetting the + * required approval means that the inherited required approval configuration or the default + * required approval ({@code Code-Review+1}) will apply. + * + * <p>In contrast to providing an empty string, providing {@code null} (or not setting the value) + * means that the required approval configuration is not updated. + */ + public String requiredApproval; + + /** + * The approvals that count as override for the code owners submit check. + * + * <p>The override approvals must be specified in the format {@code <label-name>+<label-value>}. + */ + public List<String> overrideApprovals; }
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java b/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java index 91543a6..1f2733a 100644 --- a/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java +++ b/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java
@@ -16,6 +16,8 @@ import static com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS; import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_FILE_EXTENSION; +import static com.google.gerrit.plugins.codeowners.backend.config.OverrideApprovalConfig.KEY_OVERRIDE_APPROVAL; +import static com.google.gerrit.plugins.codeowners.backend.config.RequiredApprovalConfig.KEY_REQUIRED_APPROVAL; import static com.google.gerrit.plugins.codeowners.backend.config.StatusConfig.KEY_DISABLED; import static com.google.gerrit.plugins.codeowners.backend.config.StatusConfig.KEY_DISABLED_BRANCH; @@ -118,6 +120,27 @@ SECTION_CODE_OWNERS, /* subsection= */ null, KEY_FILE_EXTENSION, input.fileExtension); } + if (input.requiredApproval != null) { + if (input.requiredApproval.isEmpty()) { + codeOwnersConfig.unset( + SECTION_CODE_OWNERS, /* subsection= */ null, KEY_REQUIRED_APPROVAL); + } else { + codeOwnersConfig.setString( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_REQUIRED_APPROVAL, + input.requiredApproval); + } + } + + if (input.overrideApprovals != null) { + codeOwnersConfig.setStringList( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_OVERRIDE_APPROVAL, + input.overrideApprovals); + } + validateConfig(projectResource.getProjectState(), codeOwnersConfig); codeOwnersProjectConfigFile.commit(metaDataUpdate);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java index 045f01b..2da6c3d 100644 --- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java +++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java
@@ -15,22 +15,26 @@ package com.google.gerrit.plugins.codeowners.acceptance.api; import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.plugins.codeowners.testing.RequiredApprovalSubject.assertThat; import static com.google.gerrit.server.project.ProjectCache.illegalState; import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static com.google.gerrit.truth.OptionalSubject.assertThat; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.gerrit.acceptance.UseClockStep; import com.google.gerrit.acceptance.testsuite.project.ProjectOperations; import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.extensions.common.LabelDefinitionInput; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT; import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo; import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInput; import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration; +import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.restapi.project.DeleteRef; import com.google.inject.Inject; @@ -189,6 +193,100 @@ } @Test + public void setRequiredApproval() throws Exception { + RequiredApproval requiredApproval = codeOwnersPluginConfiguration.getRequiredApproval(project); + assertThat(requiredApproval).hasLabelNameThat().isEqualTo("Code-Review"); + assertThat(requiredApproval).hasValueThat().isEqualTo(1); + + String otherLabel = "Other"; + LabelDefinitionInput labelInput = new LabelDefinitionInput(); + labelInput.values = ImmutableMap.of("+2", "Approval", "+1", "LGTM", " 0", "No Vote"); + gApi.projects().name(project.get()).label(otherLabel).create(labelInput).get(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.requiredApproval = otherLabel + "+2"; + CodeOwnerProjectConfigInfo updatedConfig = + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.requiredApproval.label).isEqualTo(otherLabel); + assertThat(updatedConfig.requiredApproval.value).isEqualTo(2); + requiredApproval = codeOwnersPluginConfiguration.getRequiredApproval(project); + assertThat(requiredApproval).hasLabelNameThat().isEqualTo(otherLabel); + assertThat(requiredApproval).hasValueThat().isEqualTo(2); + + input.requiredApproval = ""; + updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.requiredApproval.label).isEqualTo("Code-Review"); + assertThat(updatedConfig.requiredApproval.value).isEqualTo(1); + requiredApproval = codeOwnersPluginConfiguration.getRequiredApproval(project); + assertThat(requiredApproval).hasLabelNameThat().isEqualTo("Code-Review"); + assertThat(requiredApproval).hasValueThat().isEqualTo(1); + } + + @Test + public void setInvalidRequiredApproval() throws Exception { + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.requiredApproval = "Non-Existing-Label+2"; + BadRequestException exception = + assertThrows( + BadRequestException.class, + () -> projectCodeOwnersApiFactory.project(project).updateConfig(input)); + assertThat(exception) + .hasMessageThat() + .contains( + String.format( + "invalid config:\n" + + "* Required approval 'Non-Existing-Label+2' that is configured in" + + " code-owners.config (parameter codeOwners.requiredApproval) is invalid:" + + " Label Non-Existing-Label doesn't exist for project %s.", + project)); + } + + @Test + public void setOverrideApproval() throws Exception { + assertThat(codeOwnersPluginConfiguration.getOverrideApproval(project)).isEmpty(); + + String overrideLabel1 = "Bypass-Owners"; + String overrideLabel2 = "Owners-Override"; + createOwnersOverrideLabel(overrideLabel1); + createOwnersOverrideLabel(overrideLabel2); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.overrideApprovals = ImmutableList.of(overrideLabel1 + "+1", overrideLabel2 + "+1"); + CodeOwnerProjectConfigInfo updatedConfig = + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.overrideApproval).hasSize(2); + assertThat(updatedConfig.overrideApproval.get(0).label).isEqualTo(overrideLabel1); + assertThat(updatedConfig.overrideApproval.get(0).value).isEqualTo(1); + assertThat(updatedConfig.overrideApproval.get(1).label).isEqualTo(overrideLabel2); + assertThat(updatedConfig.overrideApproval.get(1).value).isEqualTo(1); + assertThat(codeOwnersPluginConfiguration.getOverrideApproval(project)).hasSize(2); + + input.overrideApprovals = ImmutableList.of(); + updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.overrideApproval).isNull(); + assertThat(codeOwnersPluginConfiguration.getOverrideApproval(project)).isEmpty(); + } + + @Test + public void setInvalidOverrideApproval() throws Exception { + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.overrideApprovals = ImmutableList.of("Non-Existing-Label+2"); + BadRequestException exception = + assertThrows( + BadRequestException.class, + () -> projectCodeOwnersApiFactory.project(project).updateConfig(input)); + assertThat(exception) + .hasMessageThat() + .contains( + String.format( + "invalid config:\n" + + "* Required approval 'Non-Existing-Label+2' that is configured in" + + " code-owners.config (parameter codeOwners.overrideApproval) is invalid:" + + " Label Non-Existing-Label doesn't exist for project %s.", + project)); + } + + @Test @UseClockStep public void checkCommitData() throws Exception { RevCommit head1 = projectOperations.project(project).getHead(RefNames.REFS_CONFIG);
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md index ebf1afc..bde20bd 100644 --- a/resources/Documentation/rest-api.md +++ b/resources/Documentation/rest-api.md
@@ -837,6 +837,8 @@ | `disabled` | optional | Whether the code owners functionality should be disabled/enabled for the project. | `disabled_branch` | optional | List of branches for which the code owners functionality is disabled. Can be exact refs, ref patterns or regular expressions. Overrides any existing disabled branch configuration. | `file_extension` | optional | The file extension that should be used for code owner config files in this project. +| `required_approval` | optional | The approval that is required from code owners. Must be specified in the format "\<label-name\>+\<label-value\>". If an empty string is provided the required approval configuration is unset. Unsetting the required approval means that the inherited required approval configuration or the default required approval (`Code-Review+1`) will apply. In contrast to providing an empty string, providing `null` (or not setting the value) means that the required approval configuration is not updated. +| `override_approvals` | optional | The approvals that count as override for the code owners submit check. Must be specified in the format "\<label-name\>+\<label-value\>". ---