GetCodeOwnerBranchConfig: Include a flag if no code owner config file exists yet
If a branch doesn't contain any code owner config file yet, we are in
bootstrapping mode where project owners count as code owners. Let the
frontend know about this, so that it can inform users if this is the
case. E.g. this may help users to understand why no code owners can be
suggested.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I84560336f862c0d2d8650b914732ff60016b2f72
diff --git a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
index 2cb9375..c0ae04b 100644
--- a/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
+++ b/java/com/google/gerrit/plugins/codeowners/api/CodeOwnerBranchConfigInfo.java
@@ -58,4 +58,16 @@
* <p>Not set if {@link #disabled} is {@code true}.
*/
public RequiredApprovalInfo overrideApproval;
+
+ /**
+ * Whether the branch doesn't contain any code owner config file yet.
+ *
+ * <p>If a branch doesn't contain any code owner config file yet, the projects owners are
+ * considered as code owners. Once a first code owner config file is added to the branch, the
+ * project owners are no longer code owners (unless code ownership is granted to them via the code
+ * owner config file).
+ *
+ * <p>Not set if {@code false} or if {@link #disabled} is {@code true}.
+ */
+ public Boolean noCodeOwnersDefined;
}
diff --git a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
index 737a172..72e6de0 100644
--- a/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
+++ b/java/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJson.java
@@ -32,6 +32,7 @@
import com.google.gerrit.plugins.codeowners.api.GeneralInfo;
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
import com.google.gerrit.plugins.codeowners.config.RequiredApproval;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -49,13 +50,16 @@
@Singleton
public class CodeOwnerProjectConfigJson {
private final CodeOwnersPluginConfiguration codeOwnersPluginConfiguration;
+ private final CodeOwnerConfigScanner codeOwnerConfigScanner;
private final Provider<ListBranches> listBranches;
@Inject
CodeOwnerProjectConfigJson(
CodeOwnersPluginConfiguration codeOwnersPluginConfiguration,
+ CodeOwnerConfigScanner codeOwnerConfigScanner,
Provider<ListBranches> listBranches) {
this.codeOwnersPluginConfiguration = codeOwnersPluginConfiguration;
+ this.codeOwnerConfigScanner = codeOwnerConfigScanner;
this.listBranches = listBranches;
}
@@ -93,6 +97,10 @@
info.requiredApproval = formatRequiredApprovalInfo(branchResource.getNameKey());
info.overrideApproval = formatOverrideApprovalInfo(branchResource.getNameKey());
+ boolean noCodeOwnersDefined =
+ !codeOwnerConfigScanner.containsAnyCodeOwnerConfigFile(branchResource.getBranchKey());
+ info.noCodeOwnersDefined = noCodeOwnersDefined ? noCodeOwnersDefined : null;
+
return info;
}
diff --git a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
index 20958e3..37d0d23 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/acceptance/api/GetCodeOwnerBranchConfigIT.java
@@ -75,6 +75,14 @@
@Test
public void getDefaultConfig() throws Exception {
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
assertThat(codeOwnerBranchConfigInfo.general.fileExtension).isNull();
@@ -90,6 +98,7 @@
assertThat(codeOwnerBranchConfigInfo.requiredApproval.value)
.isEqualTo(RequiredApprovalConfig.DEFAULT_VALUE);
assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
}
@Test
@@ -212,6 +221,14 @@
assertThat(codeOwnerBranchConfigInfo.general.implicitApprovals).isTrue();
}
+ @Test
+ public void getConfig_bootstrappingMode() throws Exception {
+ configureImplicitApprovals(project);
+ CodeOwnerBranchConfigInfo codeOwnerBranchConfigInfo =
+ projectCodeOwnersApiFactory.project(project).branch("master").getConfig();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isTrue();
+ }
+
private void configureFileExtension(Project.NameKey project, String fileExtension)
throws Exception {
setConfig(project, null, GeneralConfig.KEY_FILE_EXTENSION, fileExtension);
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD b/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
index 19e6cce..2c509ff 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/BUILD
@@ -6,6 +6,7 @@
deps = [
"//plugins/code-owners:code-owners__plugin",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance",
+ "//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/acceptance/testsuite",
"//plugins/code-owners/java/com/google/gerrit/plugins/codeowners/testing",
],
)
diff --git a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
index 232576a..456ebc1 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/restapi/CodeOwnerProjectConfigJsonTest.java
@@ -24,6 +24,7 @@
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.LabelType;
import com.google.gerrit.plugins.codeowners.acceptance.AbstractCodeOwnersTest;
+import com.google.gerrit.plugins.codeowners.acceptance.testsuite.CodeOwnerConfigOperations;
import com.google.gerrit.plugins.codeowners.api.BackendInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerBranchConfigInfo;
import com.google.gerrit.plugins.codeowners.api.CodeOwnerProjectConfigInfo;
@@ -31,6 +32,7 @@
import com.google.gerrit.plugins.codeowners.api.MergeCommitStrategy;
import com.google.gerrit.plugins.codeowners.api.RequiredApprovalInfo;
import com.google.gerrit.plugins.codeowners.backend.CodeOwnerBackendId;
+import com.google.gerrit.plugins.codeowners.backend.CodeOwnerConfigScanner;
import com.google.gerrit.plugins.codeowners.backend.findowners.FindOwnersBackend;
import com.google.gerrit.plugins.codeowners.backend.proto.ProtoBackend;
import com.google.gerrit.plugins.codeowners.config.CodeOwnersPluginConfiguration;
@@ -62,15 +64,19 @@
@Inject private CurrentUser currentUser;
+ private CodeOwnerConfigOperations codeOwnerConfigOperations;
private CodeOwnerProjectConfigJson codeOwnerProjectConfigJson;
private FindOwnersBackend findOwnersBackend;
private ProtoBackend protoBackend;
@Before
public void setUpCodeOwnersPlugin() throws Exception {
+ codeOwnerConfigOperations =
+ plugin.getSysInjector().getInstance(CodeOwnerConfigOperations.class);
codeOwnerProjectConfigJson =
new CodeOwnerProjectConfigJson(
codeOwnersPluginConfiguration,
+ plugin.getSysInjector().getInstance(CodeOwnerConfigScanner.class),
plugin.getSysInjector().getInstance(new Key<Provider<ListBranches>>() {}));
findOwnersBackend = plugin.getSysInjector().getInstance(FindOwnersBackend.class);
protoBackend = plugin.getSysInjector().getInstance(ProtoBackend.class);
@@ -224,6 +230,14 @@
public void formatCodeOwnerBranchConfig() throws Exception {
createOwnersOverrideLabel();
+ codeOwnerConfigOperations
+ .newCodeOwnerConfig()
+ .project(project)
+ .branch("master")
+ .folderPath("/")
+ .addCodeOwnerEmail(admin.email())
+ .create();
+
when(codeOwnersPluginConfiguration.isDisabled(any(BranchNameKey.class))).thenReturn(false);
when(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.thenReturn(findOwnersBackend);
@@ -256,6 +270,7 @@
assertThat(codeOwnerBranchConfigInfo.requiredApproval.value).isEqualTo(2);
assertThat(codeOwnerBranchConfigInfo.overrideApproval.label).isEqualTo("Owners-Override");
assertThat(codeOwnerBranchConfigInfo.overrideApproval.value).isEqualTo(1);
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
}
@Test
@@ -269,6 +284,33 @@
assertThat(codeOwnerBranchConfigInfo.backendId).isNull();
assertThat(codeOwnerBranchConfigInfo.requiredApproval).isNull();
assertThat(codeOwnerBranchConfigInfo.overrideApproval).isNull();
+ assertThat(codeOwnerBranchConfigInfo.noCodeOwnersDefined).isNull();
+ }
+
+ @Test
+ public void formatCodeOwnerBranchConfig_bootstrappingMode() 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.noCodeOwnersDefined).isTrue();
}
private ProjectResource createProjectResource() {
diff --git a/resources/Documentation/rest-api.md b/resources/Documentation/rest-api.md
index d3a28e3..2a0cb32 100644
--- a/resources/Documentation/rest-api.md
+++ b/resources/Documentation/rest-api.md
@@ -531,6 +531,7 @@
| `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`.
+| `no_code_owners_defined` | optional | Whether the branch doesn't contain any code owner config file yet. If a branch doesn't contain any code owner config file yet, the projects owners are considered as code owners. Once a first code owner config file is added to the branch, the project owners are no longer code owners (unless code ownership is granted to them via the code owner config file). Not set if `false` or if `disabled` is `true`.
---
