Add REST endpoint to get the code owner configuration for a branch
Before the frontend can display any code owner controls on the change
screen it needs to know whether the code owners functionality is enabled
for the destination branch of the change. To find out about this the
frontend currently calls the Get Code Owner Project Config REST
endpoint. The problem is that this REST endpoint returns the code owner
configuration for all branches that exist in the repository and if there
are many branches it gets slow (e.g. > 3s for projects with hundreds of
branches). This latency is too high for the frontend, however the
frontend is only interested in the code owner config of a single branch,
hence we now offer a REST endpoint to get the code owner configuration
for a branch. This REST endpoint is faster, as it doesn't need to
iterate over all branches that exist in the repository, but only needs
to return the config for that one branch.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ie9de32d9ad5debabc403a69b38009154a82a529e
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
index 8ff1222..1836ca9 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwners.java
@@ -25,6 +25,9 @@
* <p>To create an instance for a branch use {@link ProjectCodeOwners#branch(String)}.
*/
public interface BranchCodeOwners {
+ /** Returns the code owner project configuration. */
+ CodeOwnerBranchConfigInfo getConfig() throws RestApiException;
+
/** Create a request to retrieve code owner config files from the branch. */
CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException;
@@ -57,6 +60,11 @@
*/
class NotImplemented implements BranchCodeOwners {
@Override
+ public CodeOwnerBranchConfigInfo getConfig() throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
index 189b0d1..6ed89a9 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/BranchCodeOwnersImpl.java
@@ -14,7 +14,10 @@
package com.google.gerrit.plugins.codeowners.api;
+import static com.google.gerrit.server.api.ApiUtil.asRestApiException;
+
import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerBranchConfig;
import com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerConfigFiles;
import com.google.gerrit.server.project.BranchResource;
import com.google.inject.Inject;
@@ -28,18 +31,30 @@
BranchCodeOwnersImpl create(BranchResource branchResource);
}
+ private final GetCodeOwnerBranchConfig getCodeOwnerBranchConfig;
private final Provider<GetCodeOwnerConfigFiles> getCodeOwnerConfigFilesProvider;
private final BranchResource branchResource;
@Inject
public BranchCodeOwnersImpl(
+ GetCodeOwnerBranchConfig getCodeOwnerBranchConfig,
Provider<GetCodeOwnerConfigFiles> getCodeOwnerConfigFilesProvider,
@Assisted BranchResource branchResource) {
this.getCodeOwnerConfigFilesProvider = getCodeOwnerConfigFilesProvider;
+ this.getCodeOwnerBranchConfig = getCodeOwnerBranchConfig;
this.branchResource = branchResource;
}
@Override
+ public CodeOwnerBranchConfigInfo getConfig() throws RestApiException {
+ try {
+ return getCodeOwnerBranchConfig.apply(branchResource).value();
+ } catch (Exception e) {
+ throw asRestApiException("Cannot get code owner branch config", e);
+ }
+ }
+
+ @Override
public CodeOwnerConfigFilesRequest codeOwnerConfigFiles() throws RestApiException {
return new CodeOwnerConfigFilesRequest() {
@Override
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
new file mode 100644
index 0000000..2cb9375
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
@@ -0,0 +1,61 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.gerrit.plugins.codeowners.api;
+
+/**
+ * Representation of the code owner branch configuration in the REST API.
+ *
+ * <p>This class determines the JSON format of code owner branch configuration in the REST API.
+ */
+public class CodeOwnerBranchConfigInfo {
+ /**
+ * The general code owners configuration.
+ *
+ * <p>Not set if {@link #disabled} is {@code true}.
+ */
+ public GeneralInfo general;
+
+ /**
+ * Whether the code owners functionality is disabled for the branch.
+ *
+ * <p>If {@code true} the code owners API is disabled and submitting changes doesn't require code
+ * owner approvals.
+ *
+ * <p>Not set if {@code false}.
+ */
+ public Boolean disabled;
+
+ /**
+ * ID of the code owner backend that is configured for the branch.
+ *
+ * <p>Not set if {@link #disabled} is {@code true}.
+ */
+ public String backendId;
+
+ /**
+ * The approval that is required from code owners to approve the files in a change.
+ *
+ * <p>Defines which approval counts as code owner approval.
+ *
+ * <p>Not set if {@link #disabled} is {@code true}.
+ */
+ public RequiredApprovalInfo requiredApproval;
+
+ /**
+ * The approval that is required to override the code owners submit check.
+ *
+ * <p>Not set if {@link #disabled} is {@code true}.
+ */
+ public RequiredApprovalInfo overrideApproval;
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index b82e25d..737a172 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -26,6 +26,7 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.plugins.codeowners.api.BackendInfo;
+import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnersStatusInfo;
import com.google.gerrit.plugins.codeowners.api.GeneralInfo;
@@ -34,6 +35,7 @@
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.config.RequiredApproval;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.BranchResource;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.restapi.project.ListBranches;
import com.google.inject.Inject;
@@ -74,6 +76,26 @@
return info;
}
+ CodeOwnerBranchConfigInfo format(BranchResource branchResource) {
+ CodeOwnerBranchConfigInfo info = new CodeOwnerBranchConfigInfo();
+
+ boolean disabled = codeOwnersPluginConfiguration.isDisabled(branchResource.getBranchKey());
+ info.disabled = disabled ? disabled : null;
+
+ if (disabled) {
+ return info;
+ }
+
+ info.general = formatGeneralInfo(branchResource.getNameKey());
+ info.backendId =
+ CodeOwnerBackendId.getBackendId(
+ codeOwnersPluginConfiguration.getBackend(branchResource.getBranchKey()).getClass());
+ info.requiredApproval = formatRequiredApprovalInfo(branchResource.getNameKey());
+ info.overrideApproval = formatOverrideApprovalInfo(branchResource.getNameKey());
+
+ return info;
+ }
+
private GeneralInfo formatGeneralInfo(Project.NameKey projectName) {
GeneralInfo generalInfo = new GeneralInfo();
generalInfo.fileExtension =
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerBranchConfig.java b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerBranchConfig.java
new file mode 100644
index 0000000..1aba6a1
--- /dev/null
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/GetCodeOwnerBranchConfig.java
@@ -0,0 +1,49 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.restapi;
+
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.BranchResource;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import java.io.IOException;
+
+/**
+ * REST endpoint that gets the code owner branch configuration.
+ *
+ * <p>This REST endpoint handles {@code GET
+ * /projects/<project-name>/branches/<branch-name>/code_owners.branch_config} requests.
+ */
+@Singleton
+public class GetCodeOwnerBranchConfig implements RestReadView<BranchResource> {
+ private final CodeOwnerProjectConfigJson codeOwnerProjectConfigJson;
+
+ @Inject
+ public GetCodeOwnerBranchConfig(CodeOwnerProjectConfigJson codeOwnerProjectConfigJson) {
+ this.codeOwnerProjectConfigJson = codeOwnerProjectConfigJson;
+ }
+
+ @Override
+ public Response<CodeOwnerBranchConfigInfo> apply(BranchResource branchResource)
+ throws RestApiException, PermissionBackendException, IOException {
+ branchResource.getProjectState().checkStatePermitsRead();
+
+ return Response.ok(codeOwnerProjectConfigJson.format(branchResource));
+ }
+}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
index 28e43b0..38fe259 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/RestApiModule.java
@@ -30,6 +30,7 @@
get(CodeOwnerConfigsInBranchCollection.PathResource.PATH_KIND)
.to(GetCodeOwnerConfigForPathInBranch.class);
get(BRANCH_KIND, "code_owners.config_files").to(GetCodeOwnerConfigFiles.class);
+ get(BRANCH_KIND, "code_owners.branch_config").to(GetCodeOwnerBranchConfig.class);
factory(CodeOwnerJson.Factory.class);
DynamicMap.mapOf(binder(), CodeOwnersInBranchCollection.PathResource.PATH_KIND);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
new file mode 100644
index 0000000..20958e3
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
@@ -0,0 +1,288 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.acceptance.api;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.projects.ConfigInput;
+import com.google.gerrit.extensions.client.ProjectState;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
+import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
+import com.google.gerrit.plugins.codeowners.api.MergeCommitStrategy;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackend;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
+import com.google.gerrit.plugins.codeowners.config.BackendConfig;
+import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
+import com.google.gerrit.plugins.codeowners.config.GeneralConfig;
+import com.google.gerrit.plugins.codeowners.config.OverrideApprovalConfig;
+import com.google.gerrit.plugins.codeowners.config.RequiredApprovalConfig;
+import com.google.gerrit.plugins.codeowners.config.StatusConfig;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerBranchConfig} REST endpoint.
+ *
+ * <p>Further tests for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerBranchConfig} REST endpoint that require
+ * using the REST API are implemented in {@link
+ * com.google.gerrit.plugins.codeowners.acceptance.restapi.GetCodeOwnerBranchConfigRestIT}.
+ */
+public class GetCodeOwnerBranchConfigIT extends AbstractCodeOwnersIT {
+ private BackendConfig backendConfig;
+
+ @Before
+ public void setup() throws Exception {
+ backendConfig = plugin.getSysInjector().getInstance(BackendConfig.class);
+ }
+
+ @Test
+ public void cannotGetConfigForHiddenProject() throws Exception {
+ ConfigInput configInput = new ConfigInput();
+ configInput.state = ProjectState.HIDDEN;
+ gApi.projects().name(project.get()).config(configInput);
+
+ ResourceConflictException exception =
+ assertThrows(
+ ResourceConflictException.class,
+ () -> projectCodeOwnersApiFactory.project(project).branch("master").getConfig());
+ assertThat(exception).hasMessageThat().isEqualTo("project state HIDDEN does not permit read");
+ }
+
+ @Test
+ public void getDefaultConfig() throws Exception {
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.general.fileExtension).isNull();
+ assertThat(codeOwnerBranchConfigInfo.general.mergeCommitStrategy)
+ .isEqualTo(MergeCommitStrategy.ALL_CHANGED_FILES);
+ assertThat(codeOwnerBranchConfigInfo.general.implicitApprovals).isNull();
+ assertThat(codeOwnerBranchConfigInfo.general.overrideInfoUrl).isNull();
+ assertThat(codeOwnerBranchConfigInfo.disabled).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId)
+ .isEqualTo(CodeOwnerBackendId.getBackendId(backendConfig.getDefaultBackend().getClass()));
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.label)
+ .isEqualTo(RequiredApprovalConfig.DEFAULT_LABEL);
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.value)
+ .isEqualTo(RequiredApprovalConfig.DEFAULT_VALUE);
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
+ @Test
+ public void getConfigWithConfiguredFileExtension() throws Exception {
+ configureFileExtension(project, "foo");
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.general.fileExtension).isEqualTo("foo");
+ }
+
+ @Test
+ public void getConfigWithConfiguredOverrideInfoUrl() throws Exception {
+ configureOverrideInfoUrl(project, "http://foo.example.com");
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.general.overrideInfoUrl)
+ .isEqualTo("http://foo.example.com");
+ }
+
+ @Test
+ public void getConfigWithConfiguredMergeCommitStrategy() throws Exception {
+ configureMergeCommitStrategy(project, MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.general.mergeCommitStrategy)
+ .isEqualTo(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
+ }
+
+ @Test
+ public void getConfigForBranchOfDisabledProject() throws Exception {
+ disableCodeOwnersForProject(project);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.disabled).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.general).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabled", value = "true")
+ @GerritConfig(name = "plugin.code-owners.requiredApproval", value = "INVALID")
+ public void getConfigForBranchOfDisabledProject_invalidPluginConfig() throws Exception {
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.disabled).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.general).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
+ @Test
+ public void getConfigForDisabledBranch() throws Exception {
+ configureDisabledBranch(project, "refs/heads/master");
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.disabled).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.general).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
+ @Test
+ @GerritConfig(name = "plugin.code-owners.disabledBranch", value = "refs/heads/master")
+ @GerritConfig(name = "plugin.code-owners.requiredApproval", value = "INVALID")
+ public void getConfigForDisabledBranch_invalidPluginConfig() throws Exception {
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.disabled).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.general).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
+ @Test
+ public void getConfigWithConfiguredBackend() throws Exception {
+ String otherBackendId = getOtherCodeOwnerBackend(backendConfig.getDefaultBackend());
+ configureBackend(project, otherBackendId);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isEqualTo(otherBackendId);
+ }
+
+ @Test
+ public void getConfigWithConfiguredBackendForBranch() throws Exception {
+ String otherBackendId = getOtherCodeOwnerBackend(backendConfig.getDefaultBackend());
+ configureBackend(project, "refs/heads/master", otherBackendId);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isEqualTo(otherBackendId);
+ }
+
+ @Test
+ public void getConfigWithConfiguredRequiredApproval() throws Exception {
+ configureRequiredApproval(project, "Code-Review+2");
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.label).isEqualTo("Code-Review");
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.value).isEqualTo(2);
+ }
+
+ @Test
+ public void getConfigWithConfiguredOverrideApproval() throws Exception {
+ configureOverrideApproval(project, "Code-Review+2");
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.label).isEqualTo("Code-Review");
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.value).isEqualTo(2);
+ }
+
+ @Test
+ public void getConfigWithEnabledImplicitApprovals() throws Exception {
+ configureImplicitApprovals(project);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.general.implicitApprovals).isTrue();
+ }
+
+ private void configureFileExtension(Project.NameKey project, String fileExtension)
+ throws Exception {
+ setConfig(project, null, GeneralConfig.KEY_FILE_EXTENSION, fileExtension);
+ }
+
+ private void configureOverrideInfoUrl(Project.NameKey project, String overrideInfoUrl)
+ throws Exception {
+ setConfig(project, null, GeneralConfig.KEY_OVERRIDE_INFO_URL, overrideInfoUrl);
+ }
+
+ private void configureMergeCommitStrategy(
+ Project.NameKey project, MergeCommitStrategy mergeCommitStrategy) throws Exception {
+ setConfig(project, null, GeneralConfig.KEY_MERGE_COMMIT_STRATEGY, mergeCommitStrategy.name());
+ }
+
+ private void configureDisabledBranch(Project.NameKey project, String disabledBranch)
+ throws Exception {
+ setCodeOwnersConfig(project, null, StatusConfig.KEY_DISABLED_BRANCH, disabledBranch);
+ }
+
+ private void configureBackend(Project.NameKey project, String backendName) throws Exception {
+ configureBackend(project, null, backendName);
+ }
+
+ private void configureBackend(
+ Project.NameKey project, @Nullable String branch, String backendName) throws Exception {
+ setConfig(project, branch, BackendConfig.KEY_BACKEND, backendName);
+ }
+
+ private void configureRequiredApproval(Project.NameKey project, String requiredApproval)
+ throws Exception {
+ setConfig(project, null, RequiredApprovalConfig.KEY_REQUIRED_APPROVAL, requiredApproval);
+ }
+
+ private void configureOverrideApproval(Project.NameKey project, String overrideApproval)
+ throws Exception {
+ setConfig(project, null, OverrideApprovalConfig.KEY_OVERRIDE_APPROVAL, overrideApproval);
+ }
+
+ private void configureImplicitApprovals(Project.NameKey project) throws Exception {
+ setConfig(project, null, GeneralConfig.KEY_ENABLE_IMPLICIT_APPROVALS, "true");
+ }
+
+ private void setConfig(Project.NameKey project, String subsection, String key, String value)
+ throws Exception {
+ Config codeOwnersConfig = new Config();
+ codeOwnersConfig.setString(
+ CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS, subsection, key, value);
+ try (TestRepository<Repository> testRepo =
+ new TestRepository<>(repoManager.openRepository(project))) {
+ Ref ref = testRepo.getRepository().exactRef(RefNames.REFS_CONFIG);
+ RevCommit head = testRepo.getRevWalk().parseCommit(ref.getObjectId());
+ testRepo.update(
+ RefNames.REFS_CONFIG,
+ testRepo
+ .commit()
+ .parent(head)
+ .message("Configure code owner backend")
+ .add("code-owners.config", codeOwnersConfig.toText()));
+ }
+ projectCache.evict(project);
+ }
+
+ /** Returns the ID of a code owner backend that is not the given backend. */
+ private String getOtherCodeOwnerBackend(CodeOwnerBackend codeOwnerBackend) {
+ for (CodeOwnerBackendId codeOwnerBackendId : CodeOwnerBackendId.values()) {
+ if (!codeOwnerBackendId.getCodeOwnerBackendClass().equals(codeOwnerBackend.getClass())) {
+ return codeOwnerBackendId.getBackendId();
+ }
+ }
+ throw new IllegalStateException(
+ String.format("couldn't find other backend than %s", codeOwnerBackend.getClass()));
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
index 809965d..aa43cc1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/CodeOwnersRestApiBindingsIT.java
@@ -51,7 +51,8 @@
private static final ImmutableList<RestCall> BRANCH_ENDPOINTS =
ImmutableList.of(
- RestCall.get("/projects/%s/branches/%s/code-owners~code_owners.config_files"));
+ RestCall.get("/projects/%s/branches/%s/code-owners~code_owners.config_files"),
+ RestCall.get("/projects/%s/branches/%s/code-owners~code_owners.branch_config"));
private static final ImmutableList<RestCall> BRANCH_CODE_OWNER_CONFIGS_ENDPOINTS =
ImmutableList.of(RestCall.get("/projects/%s/branches/%s/code-owners~code_owners.config/%s"));
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerBranchConfigRestIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerBranchConfigRestIT.java
new file mode 100644
index 0000000..4f64c90
--- /dev/null
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/restapi/GetCodeOwnerBranchConfigRestIT.java
@@ -0,0 +1,78 @@
+// Copyright (C) 2020 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.plugins.codeowners.acceptance.restapi;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.extensions.restapi.IdString;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersIT;
+import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
+import org.junit.Test;
+
+/**
+ * Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerBranchConfig} REST endpoint that require
+ * using REST.
+ *
+ * <p>Acceptance test for the {@link
+ * com.google.gerrit.plugins.codeowners.restapi.GetCodeOwnerBranchConfig} REST endpoint that can use
+ * the Java API are implemented in {@link
+ * com.google.gerrit.plugins.codeowners.acceptance.api.GetCodeOwnerBranchConfigIT}.
+ *
+ * <p>The tests in this class do not depend on the used code owner backend, hence we do not need to
+ * extend {@link AbstractCodeOwnersIT}.
+ */
+public class GetCodeOwnerBranchConfigRestIT extends AbstractCodeOwnersTest {
+ @Test
+ @GerritConfig(name = "plugin.code-owners.backend", value = "non-existing-backend")
+ public void cannotGetStatusIfPluginConfigurationIsInvalid() throws Exception {
+ RestResponse r =
+ adminRestSession.get(
+ String.format(
+ "/projects/%s/branches/%s/code_owners.branch_config",
+ IdString.fromDecoded(project.get()), "master"));
+ r.assertConflict();
+ assertThat(r.getEntityContent())
+ .contains(
+ "Invalid configuration of the code-owners plugin. Code owner backend"
+ + " '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/branches/%s/code_owners.branch_config",
+ IdString.fromDecoded(project.get()), "master"));
+ 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()));
+ }
+}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index ee7cca1..232576a 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
import com.google.gerrit.plugins.codeowners.api.BackendInfo;
+import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnersStatusInfo;
import com.google.gerrit.plugins.codeowners.api.MergeCommitStrategy;
@@ -35,12 +36,16 @@
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.config.RequiredApproval;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.project.BranchResource;
import com.google.gerrit.server.project.ProjectResource;
import com.google.gerrit.server.restapi.project.ListBranches;
import com.google.inject.Inject;
import com.google.inject.Key;
import com.google.inject.Provider;
+import java.io.IOException;
import java.util.Optional;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
@@ -215,8 +220,67 @@
assertThat(statusInfo.disabledBranches).containsExactly("refs/heads/master");
}
+ @Test
+ public void formatCodeOwnerBranchConfig() throws Exception {
+ createOwnersOverrideLabel();
+
+ when(codeOwnersPluginConfiguration.isDisabled(any(BranchNameKey.class))).thenReturn(false);
+ when(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .thenReturn(findOwnersBackend);
+ when(codeOwnersPluginConfiguration.getFileExtension(project)).thenReturn(Optional.of("foo"));
+ when(codeOwnersPluginConfiguration.getOverrideInfoUrl(project))
+ .thenReturn(Optional.of("http://foo.example.com"));
+ when(codeOwnersPluginConfiguration.getMergeCommitStrategy(project))
+ .thenReturn(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
+ when(codeOwnersPluginConfiguration.areImplicitApprovalsEnabled(project)).thenReturn(true);
+ when(codeOwnersPluginConfiguration.getRequiredApproval(project))
+ .thenReturn(RequiredApproval.create(getDefaultCodeReviewLabel(), (short) 2));
+ when(codeOwnersPluginConfiguration.getOverrideApproval(project))
+ .thenReturn(
+ Optional.of(
+ RequiredApproval.create(
+ LabelType.withDefaultValues("Owners-Override"), (short) 1)));
+
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ codeOwnerProjectConfigJson.format(createBranchResource("refs/heads/master"));
+ assertThat(codeOwnerBranchConfigInfo.disabled).isNull();
+ assertThat(codeOwnerBranchConfigInfo.general.fileExtension).isEqualTo("foo");
+ assertThat(codeOwnerBranchConfigInfo.general.overrideInfoUrl)
+ .isEqualTo("http://foo.example.com");
+ assertThat(codeOwnerBranchConfigInfo.general.mergeCommitStrategy)
+ .isEqualTo(MergeCommitStrategy.FILES_WITH_CONFLICT_RESOLUTION);
+ assertThat(codeOwnerBranchConfigInfo.general.implicitApprovals).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.backendId)
+ .isEqualTo(CodeOwnerBackendId.FIND_OWNERS.getBackendId());
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.label).isEqualTo("Code-Review");
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval.value).isEqualTo(2);
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.label).isEqualTo("Owners-Override");
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval.value).isEqualTo(1);
+ }
+
+ @Test
+ public void formatCodeOwnerBranchConfig_disabled() throws Exception {
+ when(codeOwnersPluginConfiguration.isDisabled(any(BranchNameKey.class))).thenReturn(true);
+
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ codeOwnerProjectConfigJson.format(createBranchResource("refs/heads/master"));
+ assertThat(codeOwnerBranchConfigInfo.disabled).isTrue();
+ assertThat(codeOwnerBranchConfigInfo.general).isNull();
+ assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
+ assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ }
+
private ProjectResource createProjectResource() {
return new ProjectResource(
projectCache.get(project).orElseThrow(illegalState(project)), currentUser);
}
+
+ private BranchResource createBranchResource(String branch) throws IOException {
+ try (Repository repository = repoManager.openRepository(project)) {
+ Ref ref = repository.exactRef(branch);
+ return new BranchResource(
+ projectCache.get(project).orElseThrow(illegalState(project)), currentUser, ref);
+ }
+ }
}
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index 73bb403..d3a28e3 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -16,6 +16,11 @@
As a response a [CodeOwnerProjectConfigInfo](#code-owner-project-config-info)
entity is returned that describes the code owner project configuration.
+The response includes the configuration of all branches. If a caller is
+interested in a particular branch only, the [Get Code Owner Branch
+Config](#get-code-owner-branch-config) REST endpoint should be used instead, as
+that REST endpoint is much faster if the project contains many branches.
+
#### Request
```
@@ -125,6 +130,43 @@
## <a id="branch-endpoints">Branch Endpoints
+### <a id="get-code-owner-branch-config">Get Code Owner Branch Config
+_'GET /projects/[\{project-name\}](../../../Documentation/rest-api-projects.html#project-name)/branches/[\{branch-id\}](../../../Documentation/rest-api-projects.html#branch-id)/code_owners.branch_config'_
+
+Gets the code owner branch configuration.
+
+As a response a [CodeOwnerBranchConfigInfo](#code-owner-branch-config-info)
+entity is returned that describes the code owner branch configuration.
+
+#### Request
+
+```
+ GET /projects/foo%2Fbar/branches/master/code_owners.branch_config HTTP/1.0
+```
+
+#### Response
+
+```
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "general": {
+ "merge_commit_strategy": "ALL_CHANGED_FILES"
+ },
+ "backend_id": "find-owners",
+ "required_approval": {
+ "label": "Code-Review",
+ "value": 1
+ },
+ "override_approval": {
+ "label": "Owners-Override",
+ "value": 1
+ }
+ }
+
### <a id="list-code-owner-config-files">List Code Owner Config Files
_'GET /projects/[\{project-name\}](../../../Documentation/rest-api-projects.html#project-name)/branches/[\{branch-id\}](../../../Documentation/rest-api-projects.html#branch-id)/code_owners.config_files/'_
@@ -478,6 +520,20 @@
---
+### <a id="code-owner-branch-config-info"> CodeOwnerBranchConfigInfo
+The `CodeOwnerBranchConfigInfo` entity describes the code owner branch
+configuration.
+
+| Field Name | | Description |
+| ----------- | -------- | ----------- |
+| `general` | optional | The general code owners configuration as [GeneralInfo](#general-info) entity. Not set if `disabled` is `true`.
+| `disabled` | optional | Whether the code owners functionality is disabled for the branch. If `true` the code owners API is disabled and submitting changes doesn't require code owner approvals. Not set if `false`.
+| `backend_id`| optional | ID of the code owner backend that is configured for the branch. Not set if `disabled` is `true`.
+| `required_approval` | optional | The approval that is required from code owners to approve the files in a change as [RequiredApprovalInfo](#required-approval-info) entity. The required approval defines which approval counts as code owner approval. Not set if `disabled` is `true`.
+| `override_approval` | optional | The approval that is required to override the code owners submit check as [RequiredApprovalInfo](#required-approval-info) entity. If unset, overriding the code owners submit check is disabled. Not set if `disabled` is `true`.
+
+---
+
### <a id="code-owner-project-config-info"> CodeOwnerProjectConfigInfo
The `CodeOwnerProjectConfigInfo` entity describes the code owner project
configuration.
diff --git a/resources/Documentation/setup-guide.md b/resources/Documentation/setup-guide.md
index 507f50c..f94044f 100644
--- a/resources/Documentation/setup-guide.md
+++ b/resources/Documentation/setup-guide.md
@@ -378,43 +378,24 @@
##### <a id="checkIfEnabled">How to check if the code owners functionality is enabled for a project or branch
-To check if the code owners functionality is enabled for a project or branch,
-use the [Get Code Owner Project Config](rest-api.html#get-code-owner-project-config)
-REST endpoint and inspect the [status](rest-api.html#code-owners-status-info) in
-the response.
+To check if the code owners functionality is enabled for a single branch, use
+the [Get Code Owner Branch Config](rest-api.html#get-code-owner-branch-config)
+REST endpoint and inspect the
+[disabled](rest-api.html#code-owner-branch-config-info) field in the response
+(if it is not present, the code owners functionality is enabled).
-You can invoke the REST endpoint via `curl` from the command-line or
-alternatively open the following URL in a browser:\
+To check if the code owners functionality is enabled for a project or for
+multiple branches, use the [Get Code Owner Project
+Config](rest-api.html#get-code-owner-project-config) REST endpoint and inspect
+the [status](rest-api.html#code-owners-status-info) in the response (an empty
+status means that the code owners functionality is enabled for all branches of
+the project).
+
+You can invoke the REST endpoints via `curl` from the command-line or
+alternatively open the following URLs in a browser:\
+`https://<host>/projects/<project-name>/branches/<branch-name>/code_owners.branch_config`\
`https://<host>/projects/<project-name>/code_owners.project_config`\
-(remember to URL-encode the project-name)
-
-Example response (an empty status means that the code owners functionality is
-enabled for all branches of the project):
-
-```
- )]}'
- {
- "general": {
- "merge_commit_strategy": "ALL_CHANGED_FILES",
- },
- "status": {
- "disabled_branches": [
- "refs/meta/config"
- ]
- },
- "backend": {
- "id": "find-owners"
- },
- "required_approval": {
- "label": "Code-Review",
- "value": 1
- },
- "override_approval": {
- "label": "Owners-Override",
- "value": 1
- }
- }
-```
+(remember to URL-encode the project-name and branch-name)
---
