Allow to change more project config parameters via REST Using REST is more comfortable than editing the code-owners.config file manually. Signed-off-by: Edwin Kempin <ekempin@google.com> Change-Id: I5990a531f9d98b339fb41903a1aa7ed94ba1845c
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java index e571724..4dc69b0 100644 --- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java +++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerProjectConfigInput.java
@@ -15,6 +15,8 @@ package com.google.gerrit.plugins.codeowners.api; import com.google.gerrit.plugins.codeowners.backend.FallbackCodeOwners; +import com.google.gerrit.plugins.codeowners.common.CodeOwnerConfigValidationPolicy; +import com.google.gerrit.plugins.codeowners.common.MergeCommitStrategy; import java.util.List; /** @@ -65,4 +67,43 @@ /** Policy that controls who should own paths that have no code owners defined. */ public FallbackCodeOwners fallbackCodeOwners; + + /** Emails of users that should be code owners globally across all branches. */ + public List<String> globalCodeOwners; + + /** Strategy that defines for merge commits which files require code owner approvals. */ + public MergeCommitStrategy mergeCommitStrategy; + + /** Whether an implicit code owner approval from the last uploader is assumed. */ + public Boolean implicitApprovals; + + /** + * URL for a page that provides project/host-specific information about how to request a code + * owner override. + */ + public String overrideInfoUrl; + + /** Whether code owner config files are read-only. */ + public Boolean readOnly; + + /** Policy for validating code owner config files when a commit is received. */ + public CodeOwnerConfigValidationPolicy enableValidationOnCommitReceived; + + /** Policy for validating code owner config files when a change is submitted. */ + public CodeOwnerConfigValidationPolicy enableValidationOnSubmit; + + /** + * Whether modifications of code owner config files that newly add non-resolvable code owners + * should be rejected on commit received and submit. + */ + public Boolean rejectNonResolvableCodeOwners; + + /** + * Whether modifications of code owner config files that newly add non-resolvable imports should + * be rejected on commit received an submit. + */ + public Boolean rejectNonResolvableImports; + + /** The maximum number of paths that are included in change messages. */ + public Integer maxPathsInChangeMessages; }
diff --git a/java/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfig.java b/java/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfig.java index 8348893..3e5538c 100644 --- a/java/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfig.java +++ b/java/com/google/gerrit/plugins/codeowners/backend/config/GeneralConfig.java
@@ -79,7 +79,7 @@ @VisibleForTesting public static final String KEY_OVERRIDE_INFO_URL = "overrideInfoUrl"; - @VisibleForTesting static final int DEFAULT_MAX_PATHS_IN_CHANGE_MESSAGES = 100; + @VisibleForTesting public static final int DEFAULT_MAX_PATHS_IN_CHANGE_MESSAGES = 100; @VisibleForTesting public static final String KEY_REJECT_NON_RESOLVABLE_CODE_OWNERS =
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java b/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java index 69cf3db..597b694 100644 --- a/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java +++ b/java/com/google/gerrit/plugins/codeowners/restapi/PutCodeOwnerProjectConfig.java
@@ -15,8 +15,18 @@ package com.google.gerrit.plugins.codeowners.restapi; import static com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_ENABLE_IMPLICIT_APPROVALS; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_ENABLE_VALIDATION_ON_SUBMIT; import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_FALLBACK_CODE_OWNERS; import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_FILE_EXTENSION; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_GLOBAL_CODE_OWNER; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_MAX_PATHS_IN_CHANGE_MESSAGES; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_MERGE_COMMIT_STRATEGY; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_OVERRIDE_INFO_URL; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_READ_ONLY; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_REJECT_NON_RESOLVABLE_CODE_OWNERS; +import static com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig.KEY_REJECT_NON_RESOLVABLE_IMPORTS; 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; @@ -150,6 +160,83 @@ input.fallbackCodeOwners); } + if (input.globalCodeOwners != null) { + codeOwnersConfig.setStringList( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_GLOBAL_CODE_OWNER, + input.globalCodeOwners); + } + + if (input.mergeCommitStrategy != null) { + codeOwnersConfig.setEnum( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_MERGE_COMMIT_STRATEGY, + input.mergeCommitStrategy); + } + + if (input.implicitApprovals != null) { + codeOwnersConfig.setBoolean( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_ENABLE_IMPLICIT_APPROVALS, + input.implicitApprovals); + } + + if (input.overrideInfoUrl != null) { + codeOwnersConfig.setString( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_OVERRIDE_INFO_URL, + input.overrideInfoUrl); + } + + if (input.readOnly != null) { + codeOwnersConfig.setBoolean( + SECTION_CODE_OWNERS, /* subsection= */ null, KEY_READ_ONLY, input.readOnly); + } + + if (input.enableValidationOnCommitReceived != null) { + codeOwnersConfig.setEnum( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_ENABLE_VALIDATION_ON_COMMIT_RECEIVED, + input.enableValidationOnCommitReceived); + } + + if (input.enableValidationOnSubmit != null) { + codeOwnersConfig.setEnum( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_ENABLE_VALIDATION_ON_SUBMIT, + input.enableValidationOnSubmit); + } + + if (input.rejectNonResolvableCodeOwners != null) { + codeOwnersConfig.setBoolean( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_REJECT_NON_RESOLVABLE_CODE_OWNERS, + input.rejectNonResolvableCodeOwners); + } + + if (input.rejectNonResolvableImports != null) { + codeOwnersConfig.setBoolean( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_REJECT_NON_RESOLVABLE_IMPORTS, + input.rejectNonResolvableImports); + } + + if (input.maxPathsInChangeMessages != null) { + codeOwnersConfig.setInt( + SECTION_CODE_OWNERS, + /* subsection= */ null, + KEY_MAX_PATHS_IN_CHANGE_MESSAGES, + input.maxPathsInChangeMessages); + } + 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 11d60b7..6ce59bc 100644 --- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java +++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/PutCodeOwnerProjectConfigIT.java
@@ -14,6 +14,7 @@ package com.google.gerrit.plugins.codeowners.acceptance.api; +import static com.google.common.collect.ImmutableSet.toImmutableSet; 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; @@ -33,9 +34,13 @@ 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.CodeOwnerReference; import com.google.gerrit.plugins.codeowners.backend.FallbackCodeOwners; import com.google.gerrit.plugins.codeowners.backend.config.CodeOwnersPluginConfiguration; +import com.google.gerrit.plugins.codeowners.backend.config.GeneralConfig; import com.google.gerrit.plugins.codeowners.backend.config.RequiredApproval; +import com.google.gerrit.plugins.codeowners.common.CodeOwnerConfigValidationPolicy; +import com.google.gerrit.plugins.codeowners.common.MergeCommitStrategy; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.restapi.project.DeleteRef; import com.google.inject.Inject; @@ -308,6 +313,184 @@ } @Test + public void setGlobalCodeOwners() throws Exception { + assertThat(codeOwnersPluginConfiguration.getGlobalCodeOwners(project)).isEmpty(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.globalCodeOwners = ImmutableList.of(user.email(), "foo.bar@example.com"); + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat( + codeOwnersPluginConfiguration.getGlobalCodeOwners(project).stream() + .map(CodeOwnerReference::email) + .collect(toImmutableSet())) + .containsExactly(user.email(), "foo.bar@example.com"); + + input.globalCodeOwners = ImmutableList.of(); + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getGlobalCodeOwners(project)).isEmpty(); + } + + @Test + public void setMergeCommitStrategy() throws Exception { + assertThat(codeOwnersPluginConfiguration.getMergeCommitStrategy(project)) + .isEqualTo(MergeCommitStrategy.ALL_CHANGED_FILES); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.mergeCommitStrategy = MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION; + CodeOwnerProjectConfigInfo updatedConfig = + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.mergeCommitStrategy) + .isEqualTo(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION); + assertThat(codeOwnersPluginConfiguration.getMergeCommitStrategy(project)) + .isEqualTo(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION); + + input.mergeCommitStrategy = MergeCommitStrategy.ALL_CHANGED_FILES; + updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.mergeCommitStrategy) + .isEqualTo(MergeCommitStrategy.ALL_CHANGED_FILES); + assertThat(codeOwnersPluginConfiguration.getMergeCommitStrategy(project)) + .isEqualTo(MergeCommitStrategy.ALL_CHANGED_FILES); + } + + @Test + public void setImplicitApprovals() throws Exception { + assertThat(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).isFalse(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.implicitApprovals = true; + CodeOwnerProjectConfigInfo updatedConfig = + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.implicitApprovals).isTrue(); + assertThat(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).isTrue(); + + input.implicitApprovals = false; + updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.implicitApprovals).isNull(); + assertThat(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).isFalse(); + } + + @Test + public void setOverrideInfoUrl() throws Exception { + assertThat(codeOwnersPluginConfiguration.getOverrideInfoUrl(project)).isEmpty(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.overrideInfoUrl = "http://foo.bar"; + CodeOwnerProjectConfigInfo updatedConfig = + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.overrideInfoUrl).isEqualTo("http://foo.bar"); + assertThat(codeOwnersPluginConfiguration.getOverrideInfoUrl(project)) + .value() + .isEqualTo("http://foo.bar"); + + input.overrideInfoUrl = ""; + updatedConfig = projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(updatedConfig.general.overrideInfoUrl).isNull(); + assertThat(codeOwnersPluginConfiguration.getOverrideInfoUrl(project)).isEmpty(); + } + + @Test + public void setReadOnly() throws Exception { + assertThat(codeOwnersPluginConfiguration.areCodeOwnerConfigsReadOnly(project)).isFalse(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.readOnly = true; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.areCodeOwnerConfigsReadOnly(project)).isTrue(); + + input.readOnly = false; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.areCodeOwnerConfigsReadOnly(project)).isFalse(); + } + + @Test + public void setEnableValidationOnCommitReceived() throws Exception { + assertThat( + codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForCommitReceived( + project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.TRUE); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.enableValidationOnCommitReceived = CodeOwnerConfigValidationPolicy.FALSE; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat( + codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForCommitReceived( + project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.FALSE); + + input.enableValidationOnCommitReceived = CodeOwnerConfigValidationPolicy.TRUE; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat( + codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForCommitReceived( + project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.TRUE); + } + + @Test + public void setEnableValidationOnSubmit() throws Exception { + assertThat(codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForSubmit(project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.TRUE); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.enableValidationOnSubmit = CodeOwnerConfigValidationPolicy.FALSE; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForSubmit(project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.FALSE); + + input.enableValidationOnSubmit = CodeOwnerConfigValidationPolicy.TRUE; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getCodeOwnerConfigValidationPolicyForSubmit(project)) + .isEqualTo(CodeOwnerConfigValidationPolicy.TRUE); + } + + @Test + public void setRejectNonResolvableCodeOwners() throws Exception { + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableCodeOwners(project)).isTrue(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.rejectNonResolvableCodeOwners = false; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableCodeOwners(project)).isFalse(); + + input.rejectNonResolvableCodeOwners = true; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableCodeOwners(project)).isTrue(); + } + + @Test + public void setRejectNonResolvableImports() throws Exception { + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableImports(project)).isTrue(); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.rejectNonResolvableImports = false; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableImports(project)).isFalse(); + + input.rejectNonResolvableImports = true; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.rejectNonResolvableImports(project)).isTrue(); + } + + @Test + public void setMaxPathsInChangeMessages() throws Exception { + assertThat(codeOwnersPluginConfiguration.getMaxPathsInChangeMessages(project)) + .isEqualTo(GeneralConfig.DEFAULT_MAX_PATHS_IN_CHANGE_MESSAGES); + + CodeOwnerProjectConfigInput input = new CodeOwnerProjectConfigInput(); + input.maxPathsInChangeMessages = 10; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getMaxPathsInChangeMessages(project)).isEqualTo(10); + + input.maxPathsInChangeMessages = 0; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getMaxPathsInChangeMessages(project)).isEqualTo(0); + + input.maxPathsInChangeMessages = GeneralConfig.DEFAULT_MAX_PATHS_IN_CHANGE_MESSAGES; + projectCodeOwnersApiFactory.project(project).updateConfig(input); + assertThat(codeOwnersPluginConfiguration.getMaxPathsInChangeMessages(project)) + .isEqualTo(GeneralConfig.DEFAULT_MAX_PATHS_IN_CHANGE_MESSAGES); + } + + @Test @UseClockStep public void checkCommitData() throws Exception { RevCommit head1 = projectOperations.project(project).getHead(RefNames.REFS_CONFIG);
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md index 705c5de..5c21f29 100644 --- a/resources/Documentation/config.md +++ b/resources/Documentation/config.md
@@ -146,8 +146,8 @@ <a id="pluginCodeOwnersEnableValidationOnSubmit">plugin.@PLUGIN@.enableValidationOnSubmit</a> : Policy for validating code owner config files when a change is submitted. Allowed values are `true` (the code owner config file - validation is enabled and the submit of invalid code owner config files - is rejected), `false` (the code owner config file validation is + validation is enabled and the submission of invalid code owner config + files is rejected), `false` (the code owner config file validation is disabled, invalid code owner config files are not rejected) and `dry_run` (code owner config files are validated, but invalid code owner config files are not rejected).\
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md index 764b3d4..7507fde 100644 --- a/resources/Documentation/rest-api.md +++ b/resources/Documentation/rest-api.md
@@ -840,6 +840,16 @@ | `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\>". | `fallback_code_owners` | optional | Policy that controls who should own paths that have no code owners defined. +| `global_code_owners` | optional | List of emails of users that should be code owners globally across all branches. +| `merge_commit_strategy` | optional | Strategy that defines for merge commits which files require code owner approvals. Can be `ALL_CHANGED_FILES` or `FILES_WITH_CONFLICT_RESOLUTION` (see [mergeCommitStrategy](config.html#pluginCodeOwnersMergeCommitStrategy) for an explanation of these values). +| `implicit_approvals` | optional | Whether an implicit code owner approval from the last uploader is assumed. +| `override_info_url` | optional | URL for a page that provides project/host-specific information about how to request a code owner override. +| `read_only` | optional | Whether code owner config files are read-only. +| `enable_validation_on_commit_received` | optional | Policy for validating code owner config files when a commit is received. Allowed values are `true` (the code owner config file validation is enabled and the upload of invalid code owner config files is rejected), `false` (the code owner config file validation is disabled, invalid code owner config files are not rejected) and `dry_run` (code owner config files are validated, but invalid code owner config files are not rejected). +| `enable_validation_on_submit` | optional | Policy for validating code owner config files when a change is submitted. Allowed values are `true` (the code owner config file validation is enabled and the submission of invalid code owner config files is rejected), `false` (the code owner config file validation is disabled, invalid code owner config files are not rejected) and `dry_run` (code owner config files are validated, but invalid code owner config files are not rejected). +| `reject_non_resolvable_code_owners` | optional | Whether modifications of code owner config files that newly add non-resolvable code owners should be rejected on commit received and submit. +| `reject_non_resolvable_imports` | optional | Whether modifications of code owner config files that newly add non-resolvable imports should be rejected on commit received an submit. +| `max_paths_in_change_messages` | optional | The maximum number of paths that are included in change messages. Setting the value to `0` disables including owned paths into change messages. ---